Smithsonian#20 Next refid is actually the next refid (Smithsonian#24)

.github/workflows/codescan.yml

@@ -0,0 +1,32 @@
+name: Code Scan
+
+on:
+  pull_request:
+    branches:
+      - main
+  push:
+
+jobs:
+  backend_plugins:
+    runs-on: ubuntu-latest
+    env:
+      PROD_ARCHIVESSPACE_VERSION: v3.3.1
+
+    steps:
+    - name: Checkout ArchivesSpace
+      uses: actions/checkout@v4
+      with:
+        ref: ${{ env.PROD_ARCHIVESSPACE_VERSION }}
+        repository: Smithsonian/archivesspace
+
+    - name: Checkout plugin
+      uses: actions/checkout@v4
+      with:
+        path: ${{ github.event.repository.name }}
+
+    - name: Copy plugin to ArchivesSpace
+      run: |
+        cp -r ${{ github.workspace }}/${{ github.event.repository.name }} ${{ github.workspace }}/plugins
+    - name: Run Rubocop
+      run: |
+        ./build/run rubocop -Ddir="plugins/${{ github.event.repository.name }}"

.rubocop.yml

@@ -0,0 +1,9 @@
+inherit_from: ../../.rubocop.yml
+
+inherit_mode:
+  merge:
+    - Include
+
+AllCops:
+  Include:
+    - .

README.md

@@ -25,12 +25,12 @@ The second migration `migrations/002_seed_initial_values.rb` will do the work of
 
 ```
 resource, next_ref_id
-<my resource number>, <my last used refid>
+<my resource number>, <my next refid>
 7572, 621
 43363, 10
 ```
 
-You can name this file whatever you would like, including the date is generally a good idea.  While it can be saved anywhere in the plugin directory, storing it in `migrations/csvs` will make it simpler to call the file when the migration is run.
+You can name this file whatever you would like, however including the date is generally a good idea.  While it can be saved anywhere in the plugin directory, storing it in `migrations/csvs` will make it simpler to call the file when the migration is run.
 
 To run the migration and seed this data:
 
@@ -78,7 +78,7 @@ If an error is encountered during a data-altering migration (e.g. seeding initia
 
 ## Updating Refids through the UI
 
-Should one or a small number of refids need to be regenerated in the UI, a "Regenerate Ref ID?" checkbox has been added to the archival object form.  This checkbox is currently only visible to system administrators.  When checked, the same logic that autogenerates refids on new records will be called on the save of an existing archival object record.  Since this logic concatenates a resource's EADID alongside a +1 of the refid stored in the `caas_aspace_refid` table for that resource, this manual process can be useful in the case where an EADID has changed and/or if an unexpected refid collision is anticipated.
+Should one or a small number of refids need to be regenerated in the UI, a "Regenerate Ref ID?" checkbox has been added to the archival object form.  This checkbox is currently only visible to system administrators.  When checked, the same logic that autogenerates refids on new records will be called on the save of an existing archival object record.  Since this logic concatenates a resource's EADID alongside the `next_refid` stored in the `caas_aspace_refid` table for that resource, this manual process can be useful in the case where an EADID has changed and/or if an unexpected refid collision is anticipated.
 
 ## Running Tests
 

backend/controllers/caas_next_refid.rb

@@ -6,18 +6,19 @@ class ArchivesSpaceService < Sinatra::Base
     .permissions([])
     .returns([200, "{'resource_id', 'ID', 'next_refid', N}"]) \
   do
-    current_refid = CaasAspaceRefid.find(resource_id: params[:resource_id])
-    incremented_id = !current_refid.nil? ? current_refid.next_refid + 1 : 1
-    if !current_refid.nil?
-      new_refid_record = current_refid.update(next_refid: incremented_id)
-      json = CaasAspaceRefid.to_jsonmodel(new_refid_record.id)
-      handle_update(CaasAspaceRefid, current_refid.id, json)
+    existing_refid_record = CaasAspaceRefid.find(resource_id: params[:resource_id])
+    if existing_refid_record
+      incremented_refid = (existing_refid_record&.next_refid || 0) + 1
+      existing_refid_record.update(next_refid: incremented_refid)
+      json = CaasAspaceRefid.to_jsonmodel(existing_refid_record.id)
+      handle_update(CaasAspaceRefid, existing_refid_record.id, json)
     else
-      CaasAspaceRefid.create_from_json(JSONModel(:caas_next_refid).from_hash({:resource_id => params[:resource_id],
-                                                                              :next_refid => incremented_id }))
+      json = JSONModel(:caas_next_refid).from_hash({ :resource_id => params[:resource_id],
+                                                     :next_refid => 2 })
+      handle_create(CaasAspaceRefid, json)
     end
 
-    json_response(:resource_id => params[:resource_id], :next_refid => incremented_id)
+    json_response(CaasAspaceRefid.to_jsonmodel(CaasAspaceRefid.find(resource_id: params[:resource_id]).id))
   end
 
   Endpoint.get('/plugins/caas_next_refid/find_by_uri')

backend/model/archival_object.rb

@@ -1,30 +1,35 @@
 require 'net/http'
 require 'date'
 
-def generate_ref_id(resource_id)
+def generate_ref_id(resource)
+  next_refid = get_next_refid(resource)
+
   begin
-    url = URI.parse(AppConfig[:backend_url] + "/plugins/caas_next_refid?resource_id=#{resource_id}")
+    url = URI.parse(AppConfig[:backend_url] + "/plugins/caas_next_refid?resource_id=#{resource.id}")
     request = Net::HTTP::Post.new(url.to_s)
-    response = Net::HTTP.start(url.host, url.port) do |http|
+    Net::HTTP.start(url.host, url.port) do |http|
       http.request(request)
     end
-    refid = JSON(response.body)['next_refid']
+    next_refid
   rescue
-    refid = DateTime.now.strftime('%Q')
+    return DateTime.now.strftime('%Q')
   end
-  refid
+end
+
+def get_next_refid(resource)
+  resource['caas_next_refid'] ? resource['caas_next_refid']['next_refid'] : 1
 end
 
 ArchivalObject.auto_generate(property: :ref_id,
                              generator: proc do |json|
                                resource = Resource.to_jsonmodel(JSONModel::JSONModel(:resource).id_for(json['resource']['ref']))
-                               "#{resource['ead_id']}_ref#{generate_ref_id(resource['id'])}"
+                               "#{resource['ead_id']}_ref#{generate_ref_id(resource)}"
                              end,
                              only_on_create: true)
 
 ArchivalObject.auto_generate(property: :ref_id,
                              generator: proc do |json|
                                resource = Resource.to_jsonmodel(JSONModel::JSONModel(:resource).id_for(json['resource']['ref']))
-                               "#{resource['ead_id']}_ref#{generate_ref_id(resource['id'])}"
+                               "#{resource['ead_id']}_ref#{generate_ref_id(resource)}"
                              end,
                              only_if: proc { |json| json['caas_regenerate_ref_id'] })

backend/spec/controller_caas_next_refid_spec.rb

@@ -15,11 +15,33 @@
     context 'when a resource_id is provided' do
       let(:resource) { create_resource }
 
-      it 'creates a next_refid' do
-        post '/plugins/caas_next_refid', params = { "resource_id" => resource.id}
+      context 'when no caas_next_refid exists' do
+        it 'creates a next_refid with a starting increment of 2' do
+          post '/plugins/caas_next_refid', params = { "resource_id" => resource.id}
 
-        expect(last_response).to be_ok
-        expect(last_response.status).to eq(200)
+          expect(last_response).to be_ok
+          expect(last_response.status).to eq(200)
+          expect(JSON(last_response.body)['next_refid']).to eq(2)
+        end
+      end
+
+      context 'when a caas_next_refid exists' do
+        let(:caas_next_refid) do
+          JSONModel(:caas_next_refid).from_hash({ resource_id: resource.id,
+                                                  next_refid: 40 })
+        end
+
+        before do
+          CaasAspaceRefid.create_from_json(caas_next_refid)
+        end
+
+        it 'updates the next_refid by 1' do
+          post '/plugins/caas_next_refid', params = { "resource_id" => resource.id}
+
+          expect(last_response).to be_ok
+          expect(last_response.status).to eq(200)
+          expect(JSON(last_response.body)['next_refid']).to eq(41)
+        end
       end
     end
   end
@@ -49,7 +71,7 @@
 
         expect(last_response).to be_ok
         expect(last_response.status).to eq(200)
-        expect(last_response.body).to match(/"next_refid":1/)
+        expect(last_response.body).to match(/"next_refid":2/)
       end
 
 

backend/spec/model_archival_object_spec.rb

@@ -1,54 +1,166 @@
 require 'spec_helper'
 
 describe 'ArchivalObject model' do
-  let(:resource) { create_resource({ :ead_id => 'my.eadid' }) }
-  let(:archival_object) do
-    build(:json_archival_object,
-           'ref_id' => nil,
-           'resource': {
-             'ref': "/repositories/2/resources/#{resource.id}"
-           }
-          )
-  end
+  let(:resource) { create_resource({ ead_id: 'my.eadid' }) }
+  let(:resource_json) { Resource.to_jsonmodel(resource.id) }
 
-  context 'when the next_refid endpoint returns an id' do
-    let(:body) { {"resource_id": resource.id, "next_refid": 40} }
+  context 'when the next_refid endpoint responds' do
     let(:response) { instance_double(Net::HTTPResponse) }
 
     before do
       allow(Net::HTTP).to receive(:start).and_return(response)
-      allow(response).to receive(:body).and_return(body.to_json)
     end
 
-    describe '#generate_ref_id' do
-      it 'returns the refid' do
-         expect(generate_ref_id(resource.id)).to eq(40)
-       end
+    context 'when a CaasAspaceRefid record exists' do
+      let(:caas_next_refid) do
+        JSONModel(:caas_next_refid).from_hash({ resource_id: resource.id,
+                                                next_refid: 40 })
+      end
+
+      before do
+        CaasAspaceRefid.create_from_json(caas_next_refid)
+      end
+
+      describe '#generate_ref_id' do
+        it 'returns the existing refid' do
+          expect(generate_ref_id(resource_json)).to eq(40)
+        end
+      end
+
+      describe '#auto_generate' do
+        context 'when the archival object is new' do
+          let(:archival_object) do
+            create_archival_object({ ref_id: nil,
+                                     resource: { ref: "/repositories/2/resources/#{resource.id}" } })
+          end
+
+          it 'calls the caas_next_refid endpoint' do
+            archival_object
+
+            expect(Net::HTTP).to have_received(:start)
+          end
+
+          it 'auto generates the ref_id' do
+            expect(archival_object.ref_id).to eq('my.eadid_ref40')
+          end
+        end
+
+        context 'when an existing archival object is updated with caas_regenerate_ref_id set to true' do
+          let(:archival_object) do
+            create_archival_object({ ref_id: '123',
+                                     caas_regenerate_ref_id: true,
+                                     resource: { ref: "/repositories/2/resources/#{resource.id}" } })
+          end
+
+          it 'calls the caas_next_refid endpoint' do
+            archival_object
+
+            expect(Net::HTTP).to have_received(:start)
+          end
+
+          it 'auto generates ref_id' do
+            archival_object.save
+
+            expect(archival_object.ref_id).to eq('my.eadid_ref40')
+          end
+        end
+      end
     end
 
-    describe '#auto_generate' do
-      it 'auto generates ref_id' do
-        archival_object.save
+    context 'when a CaasAspaceRefid does not exist' do
+      describe '#generate_ref_id' do
+        it 'returns 1' do
+          expect(generate_ref_id(resource_json)).to eq(1)
+        end
+      end
+
+      describe '#auto_generate' do
+        context 'when the archival object is new' do
+          let(:archival_object) do
+            create_archival_object({ ref_id: nil,
+                                     resource: { ref: "/repositories/2/resources/#{resource.id}" } })
+          end
+
+          it 'calls the caas_next_refid endpoint' do
+            archival_object
+
+            expect(Net::HTTP).to have_received(:start)
+          end
+
+          it 'auto generates the ref_id' do
+            expect(archival_object.ref_id).to eq('my.eadid_ref1')
+          end
+        end
 
-        expect(archival_object.ref_id).to eq('my.eadid_ref40')
+        context 'when an existing archival object is updated with caas_regenerate_ref_id set to true' do
+          let(:archival_object) do
+            create_archival_object({ ref_id: '123',
+                                     caas_regenerate_ref_id: true,
+                                     resource: { ref: "/repositories/2/resources/#{resource.id}" } })
+          end
+
+          it 'calls the caas_next_refid endpoint' do
+            archival_object
+
+            expect(Net::HTTP).to have_received(:start)
+          end
+
+          it 'auto generates ref_id' do
+            archival_object.save
+
+            expect(archival_object.ref_id).to eq('my.eadid_ref1')
+          end
+        end
       end
     end
   end
 
   context 'when the next_refid endpoint fails to return a ref_id' do
     let(:refid_fallback) { DateTime.now.strftime('%s')[0..-2] }
 
+    before do
+      allow(Net::HTTP).to receive(:start).and_call_original
+    end
+
     describe '#generate_ref_id' do
       it 'returns a unique date string' do
-         expect(generate_ref_id(resource.id)).to start_with(refid_fallback)
+         expect(generate_ref_id(resource_json)).to start_with(refid_fallback)
        end
     end
 
     describe '#auto_generate' do
-      it 'auto generates ref_id from the unique date string' do
-        archival_object.save
+      context 'when archival object is new' do
+        let(:archival_object) do
+          create_archival_object({ ref_id: nil,
+                                  resource: { ref: "/repositories/2/resources/#{resource.id}" } })
+        end
+
+        it 'calls the caas_next_refid endpoint' do
+          archival_object
+
+          expect(Net::HTTP).to have_received(:start)
+        end
+
+        it 'auto generates ref_id from the unique date string' do
+          expect(archival_object.ref_id).to start_with("my.eadid_ref#{refid_fallback}")
+        end
+      end
+
+      context 'when archival object updated with caas_regenerate_ref_id set to true' do
+        let(:archival_object) do
+          create_archival_object({ caas_regenerate_ref_id: true,
+                                   resource: { ref: "/repositories/2/resources/#{resource.id}" } })
+        end
+
+        it 'calls the caas_next_refid endpoint' do
+          archival_object
+
+          expect(Net::HTTP).to have_received(:start)
+        end
 
-        expect(archival_object.ref_id).to start_with("my.eadid_ref#{refid_fallback}")
+        it 'auto generates ref_id' do
+          expect(archival_object.ref_id).to start_with("my.eadid_ref#{refid_fallback}")
+        end
       end
     end
   end