Merge pull request #6 from o19s/bugfix-3-rating-docs

base64 encode document ids that contain a period.  fixes #3

app/assets/javascripts/services/ratingsStoreSvc.js

@@ -4,6 +4,10 @@
 // sent down from the backend. It comes in when the query
 // is initially retrieved and managed here.
 //
+// For documents whose id is a simple string, we pass that
+// back and forth.  However, if the id of the document is
+// a URL or contains a "." character, then we do escaping.
+//
 angular.module('QuepidApp')
   .service('ratingsStoreSvc', [
     '$http',
@@ -19,8 +23,7 @@ angular.module('QuepidApp')
 
         var path      = function(docId) {
           var id = docId;
-
-          if ( /http/.test(docId) ) {
+          if ( /http/.test(docId) || /\./.test(docId)) {
             id = btoa(docId);
           }
 

app/controllers/api/v1/queries/ratings_controller.rb

@@ -34,7 +34,7 @@ def rating_params
         def id_base64? id
           id.is_a?(String) &&
             Base64.strict_encode64(Base64.strict_decode64(id)) == id &&
-            uri?(Base64.strict_decode64(id))
+            (uri?(Base64.strict_decode64(id)) or contains_period?(Base64.strict_decode64(id)))
         rescue ArgumentError
           false
         end
@@ -48,6 +48,10 @@ def uri? string
           false
         end
 
+        def contains_period? string
+          return string.include?('.')
+        end
+
         def decode_id
           @id = params[:doc_id]
 

spec/javascripts/angular/services/ratingsStoreSvc_spec.js

@@ -48,6 +48,22 @@ describe('Service: Ratingsstoresvc', function () {
     expect(ratingsStore.getRating('file://foo/bar')).toBe(10);
     $httpBackend.verifyNoOutstandingExpectation();
   });
+  it('should base 64 and urlencode when POSTIng rating w id is URL', function() {
+    var ratingsStore = ratingsStoreSvc.createRatingsStore(0, 1, {});
+    $httpBackend.expectPUT('/api/cases/0/queries/1/ratings/aHR0cDovL3d3dy5leGFtcGxlLmNvbS9kb2MvMQ%3D%3D').respond(200, {});
+    ratingsStore.rateDocument('http://www.example.com/doc/1', 10);
+    $httpBackend.flush();
+    expect(ratingsStore.getRating('http://www.example.com/doc/1')).toBe(10);
+    $httpBackend.verifyNoOutstandingExpectation();
+  });
+  it('should base 64 and urlencode when POSTIng rating w id containing a period', function() {
+    var ratingsStore = ratingsStoreSvc.createRatingsStore(0, 1, {});
+    $httpBackend.expectPUT('/api/cases/0/queries/1/ratings/bXlkb2MucGRm').respond(200, {});
+    ratingsStore.rateDocument('mydoc.pdf', 10);
+    $httpBackend.flush();
+    expect(ratingsStore.getRating('mydoc.pdf')).toBe(10);
+    $httpBackend.verifyNoOutstandingExpectation();
+  });
 
   it('should urlencode when DELETING rating', function() {
     var ratingsStore = ratingsStoreSvc.createRatingsStore(0, 1, {});

test/controllers/api/v1/queries/ratings_controller_test.rb

@@ -72,7 +72,7 @@ class RatingsControllerTest < ActionController::TestCase
             assert_equal count, 1
           end
 
-          test 'works with a url as the id' do
+          test "works with a url as the id" do
             doc_id     = 'https%3A%2F%2Fexample.com%2Frelative-path'
             encoded_id = Base64.strict_encode64(doc_id)
 
@@ -134,7 +134,25 @@ class RatingsControllerTest < ActionController::TestCase
             assert_equal count, 1
           end
 
-          describe 'analytics' do
+          test "works with a document id that contains a period" do
+            doc_id = 'mydoc.pdf'
+
+            put :update, case_id: acase.id, query_id: query.id, doc_id: doc_id, rating: 5
+
+            assert_response :ok
+
+            data = JSON.parse(response.body)
+
+            assert_equal data['rating'],    5
+            assert_equal data['doc_id'],    doc_id
+            assert_equal data['query_id'],  query.id
+
+            count = query.ratings.where(doc_id: doc_id).count
+
+            assert_equal count, 1
+          end
+
+          describe "analytics" do
             test 'posts event' do
               expects_any_ga_event_call
 

test/integration/api/rating_documents_flow_test.rb

@@ -0,0 +1,26 @@
+require 'test_helper'
+
+class RatingDocumentsFlowTest < ActionDispatch::IntegrationTest
+
+  test "can rate documents that have various formatted document ids" do
+
+    # asserts the route for normal doc id's works.
+    assert_routing(
+      { method: 'put', path: "/api/cases/44/queries/62/ratings/99" },
+      { controller: "api/v1/queries/ratings", action: "update", format: :json, case_id: "44", "query_id": "62", doc_id: "99" }
+    )
+
+    # make sure using other non numeric identifiers works.
+    assert_routing(
+      { method: 'put', path: "/api/cases/44/queries/62/ratings/mydoc" },
+      { controller: "api/v1/queries/ratings", action: "update", format: :json, case_id: "44", "query_id": "62", doc_id: "mydoc" }
+    )
+
+    # A period in the doc_id should work, however Rails assumes that post the dot is the format type.   We deal with this by
+    # Base64 encoding in the Angular front end and decoding in ratings controller.
+    assert_routing(
+      { method: 'put', path: "/api/cases/44/queries/62/ratings/mydoc.pdf" },
+      { controller: "api/v1/queries/ratings", action: "update", "format": "pdf", case_id: "44", "query_id": "62", doc_id: "mydoc" }
+    )
+  end
+end