Fix approver bug (#804)

* Strip format in uri when determining login type during auth so it doesn't default to Author.  Also, only link committee members to approvers when directing to approver index so the linking doesn't happen whne datatables are loading

* Rubocop.  Also, changed link_to in special committee page to button_to to ensure a post is sent

* Fixed special committee integration tests to look for button rather than link

app/controllers/approver/approvers_controller.rb

@@ -6,7 +6,7 @@ class Approver::ApproversController < ApproverController
 
   def index
     @approver = current_approver
-    ApproversService.new(current_approver).update_committee_w_access_id
+    ApproversService.new(@approver).update_committee_w_access_id if request.format.html?
     @committee_members = @approver.committee_members.select do |n|
       n if n.approval_started_at.present? && n.submission.status_behavior.beyond_collecting_final_submission_files?
     end

app/views/special_committee/main.html.erb

@@ -2,7 +2,7 @@
   <div>
     <div class="h2">Already have or created a Penn State OneID account?</div>
     <br>
-    <p><%= link_to "Proceed to ETD My Reviews Page", { controller: :special_committee, action: :advance_to_reviews }, method: :post, class: "btn btn-main" %></p>
+    <p><%= button_to "Proceed to ETD My Reviews Page", { controller: :special_committee, action: :advance_to_reviews }, method: :post, class: "btn btn-main" %></p>
   </div>
 <br>
   <div>

lib/devise/strategies/oidc_authenticatable.rb

@@ -89,7 +89,7 @@ def determine_login_type(uri)
         this_uri = uri.split('/')
         return 'Author' unless this_uri.length > 1
 
-        this_uri = uri.split('/')[1].camelcase
+        this_uri = uri.split('/')[1].gsub(/\.\w+$/, '').camelcase
         this_uri = 'Author' unless ['Author', 'Admin', 'Approver'].include? this_uri
         this_uri
       end

spec/controllers/approver/approvers_controller_spec.rb

@@ -28,6 +28,16 @@
       expect(CommitteeMember.find(committee_member1.id).approver_id).to eq approver.id
       expect(CommitteeMember.find(committee_member2.id).approver_id).to eq approver.id
     end
+
+    context 'when format is something other than html' do
+      it 'does not link committee member records' do
+        committee_member1 = FactoryBot.create :committee_member, access_id: 'approverflow'
+        get :index, params: { format: :json }
+        expect(committee_member1.approver_id).to eq nil
+        expect(CommitteeMember.find(committee_member1.id).approver_id).to eq nil
+        expect(Approver.find_by(access_id: 'approverflow').committee_members).to be_empty
+      end
+    end
   end
 
   describe '#edit' do

spec/integration/special_committee_spec.rb

@@ -26,7 +26,7 @@
       expect(page).to have_content('New to Penn State?')
       expect(page).to have_link('Create Your Penn State Account', href: 'https://accounts.psu.edu/create/new')
       expect(page).to have_content('Already have or created a Penn State OneID account?')
-      expect(page).to have_link('Proceed to ETD My Reviews Page')
+      expect(page).to have_button('Proceed to ETD My Reviews Page')
     end
 
     it 'marries an approver and multiple committee member records via token when clicking advance button', js: true do
@@ -39,7 +39,7 @@
       allow_any_instance_of(LdapUniversityDirectory).to receive(:exists?).and_return(true)
       oidc_authorize_approver
       expect(Approver.find_by(access_id: 'approverflow').committee_members.count).to eq 0
-      find(:xpath, "//a[@href='/special_committee/1/advance_to_reviews']").click
+      click_button("Proceed to ETD My Reviews Page")
       expect(Approver.find_by(access_id: 'approverflow').committee_members.count).to eq 2
       expect(Approver.find_by(access_id: 'approverflow').committee_members.first.access_id).to eq 'approverflow'
       expect(Approver.find_by(access_id: 'approverflow').committee_members.second.access_id).to eq 'approverflow'
@@ -51,7 +51,7 @@
 
     it 'does not marry an approver and committee member record via token when clicking advance button', js: true do
       visit '/special_committee/1'
-      find(:xpath, "//a[@href='/special_committee/1/advance_to_reviews']").click
+      click_button("Proceed to ETD My Reviews Page")
       expect { Approver.find_by(access_id: 'approverflow').committee_members.count }.to raise_error NoMethodError
       expect(CommitteeMemberToken.find(committee_member_token.id)).to eq committee_member_token
     end
@@ -62,7 +62,7 @@
       allow_any_instance_of(LdapUniversityDirectory).to receive(:exists?).and_return(true)
       oidc_authorize_approver
       expect(Approver.find_by(access_id: 'approverflow').committee_members.count).to eq 0
-      find(:xpath, "//a[@href='/special_committee/1/advance_to_reviews']").click
+      click_button("Proceed to ETD My Reviews Page")
       expect(Approver.find_by(access_id: 'approverflow').committee_members.count).to eq 1
       expect(Approver.find_by(access_id: 'approverflow').committee_members.first.access_id).to eq 'approverflow'
       expect { CommitteeMemberToken.find(committee_member_token.id) }.to raise_error ActiveRecord::RecordNotFound

spec/lib/devise/oidc_authenticatable_spec.rb

@@ -7,6 +7,7 @@
   subject { described_class.new(nil) }
 
   before { allow(subject).to receive(:request).and_return(request) }
+  let(:request) {}
 
   describe 'authenticate!' do
     context 'when author' do
@@ -143,4 +144,11 @@
       end
     end
   end
+
+  context 'when format is appended to the end of the url' do
+    it 'strips format to determine_login_type' do
+      request_uri = '/approver.json'
+      expect(subject.send(:determine_login_type, request_uri)).to eq('Approver')
+    end
+  end
 end