robust handling of number prefixed project identifier on query parsing

lib/api/v3/queries/query_representer.rb

@@ -44,10 +44,13 @@ module API
                             setter: ->(fragment:, **) {
                               id = id_from_href "projects", fragment['href']
 
-                              id = if id.to_i.nonzero?
+                              # In case an identifier is provided, which might
+                              # start with numbers, the id needs to be looked up
+                              # in the DB.
+                              id = if id.to_i.to_s == id
                                      id # return numerical ID
                                    else
-                                     Project.where(identifier: id).pluck(:id).first # lookup Project by identifier
+                                     Project.where(identifier: id).pick(:id) # lookup Project by identifier
                                    end
 
                               represented.project_id = id if id

spec/lib/api/v3/queries/query_representer_parsing_spec.rb

@@ -28,24 +28,23 @@
 
 require 'spec_helper'
 
-describe ::API::V3::Queries::QueryRepresenter do
+describe ::API::V3::Queries::QueryRepresenter, 'parsing' do
   include ::API::V3::Utilities::PathHelper
 
   let(:query) { FactoryBot.build_stubbed(:query, project: project) }
   let(:project) { FactoryBot.build_stubbed(:project) }
-  let(:user) { double('current_user') }
   let(:representer) do
-    described_class.new(query, current_user: user, embed_links: true)
+    described_class.new(query, current_user: current_user, embed_links: true)
   end
 
   let(:permissions) { [] }
 
   let(:policy) do
-    policy_stub = double('policy stub')
+    policy_stub = instance_double(QueryPolicy)
 
     allow(QueryPolicy)
       .to receive(:new)
-      .with(user)
+      .with(current_user)
       .and_return(policy_stub)
 
     allow(policy_stub)
@@ -60,13 +59,15 @@ describe ::API::V3::Queries::QueryRepresenter do
     end
   end
 
+  current_user { FactoryBot.build_stubbed(:user) }
+
   before do
     policy
   end
 
   subject { representer.from_hash request_body }
 
-  describe 'parsing empty group_by (Regression #25606)' do
+  describe 'empty group_by (Regression #25606)' do
     before do
       query.group_by = 'project'
     end
@@ -79,7 +80,7 @@ describe ::API::V3::Queries::QueryRepresenter do
       }
     end
 
-    it 'should unset group_by' do
+    it 'unsets group_by' do
       expect(query).to be_grouped
       expect(query.group_by).to eq('project')
 
@@ -87,7 +88,7 @@ describe ::API::V3::Queries::QueryRepresenter do
     end
   end
 
-  describe 'parsing highlighted_attributes', with_ee: [:conditional_highlighting] do
+  describe 'highlighted_attributes', with_ee: [:conditional_highlighting] do
     let(:request_body) do
       {
         '_links' => {
@@ -96,12 +97,12 @@ describe ::API::V3::Queries::QueryRepresenter do
       }
     end
 
-    it 'should set highlighted_attributes' do
+    it 'sets highlighted_attributes' do
       expect(subject.highlighted_attributes).to eq(%i{type})
     end
   end
 
-  describe 'parsing ordered work packages' do
+  describe 'ordered work packages' do
     let(:request_body) do
       {
         'orderedWorkPackages' => {
@@ -116,22 +117,81 @@ describe ::API::V3::Queries::QueryRepresenter do
       allow(query).to receive(:new_record?).and_return(new_record)
     end
 
-    context 'assuming query is new' do
+    context 'if query is new' do
       let(:new_record) { true }
-      it 'should set ordered_work_packages' do
+
+      it 'sets ordered_work_packages' do
         order = subject.ordered_work_packages.map { |el| [el.work_package_id, el.position] }
         expect(order).to match_array [[50, 0], [38, 1234], [102, 81234123]]
       end
     end
 
-    context 'assuming query is not new' do
+    context 'if query is not new' do
       let(:new_record) { false }
 
-      it 'should set ordered_work_packages' do
-        expect(query)
-          .not_to receive(:ordered_work_packages)
+      it 'sets ordered_work_packages' do
+        allow(query)
+          .to receive(:ordered_work_packages)
 
         subject
+
+        expect(query)
+          .not_to have_received(:ordered_work_packages)
+      end
+    end
+  end
+
+  describe 'project' do
+    let(:query) { FactoryBot.build_stubbed(:query, project: nil) }
+
+    let(:request_body) do
+      {
+        '_links' => {
+          'project' => {
+            'href' => "/api/v3/projects/#{project_id}"
+          }
+        }
+      }
+    end
+
+    before do
+      scope = instance_double(ActiveRecord::Relation)
+
+      allow(Project)
+        .to receive(:where)
+              .with(identifier: project_id)
+              .and_return(scope)
+      allow(scope)
+        .to receive(:pick)
+              .with(:id)
+              .and_return(project.id)
+    end
+
+    context 'for a number only id' do
+      let(:project_id) { project.id }
+
+      it 'sets the project_id accordingly' do
+        expect(subject.project_id)
+          .to eql project.id
+      end
+    end
+
+    context 'for a text only id (identifier)' do
+      let(:project_id) { project.identifier }
+
+      it 'deduces the id for the project_id accordingly' do
+        expect(subject.project_id)
+          .to eql project.id
+      end
+    end
+
+    context 'for a text starting with numbers (identifier)' do
+      let(:project) { FactoryBot.build_stubbed(:project, identifier: '5555-numbered-identifier') }
+      let(:project_id) { project.identifier }
+
+      it 'deduces the id for the project_id accordingly' do
+        expect(subject.project_id)
+          .to eql project.id
       end
     end
   end