Manually fix new unsafe Rubcop offences #4735

* Requires fixing some specs as some elements became ambiguous due to
preferring more generic `click_on` Capybara helper instead of `click_link`
and `click_button`
* Solved by searching within `form` - alternatively, we could use
test_ids. Probably a more robust approach, but preferred not having
to touch views for this pass

lib/kramdown/converter/odin_html.rb

@@ -17,10 +17,9 @@ def convert_img(element, _indent)
       def convert_a(element, indent)
         if element.attr['href'].starts_with?('http')
           element.attr.merge!(EXTERNAL_LINK_ATTRIBUTES)
-          super
-        else
-          super
         end
+
+        super
       end
 
       def convert_header(element, indent)

spec/system/admin/announcements_spec.rb

@@ -9,12 +9,12 @@
   context 'when creating a new announcement' do
     it 'displays the announcement message and expires at on the index page' do
       visit admin_announcements_path
-      click_link('New announcement')
+      click_on('New announcement')
 
       fill_in :announcement_message, with: 'Test Message'
       fill_in :announcement_learn_more_url, with: 'https://example.com'
       fill_in :announcement_expires_at, with: 10.days.from_now.to_date.to_fs(:db)
-      click_button('Save')
+      click_on('Save')
 
       visit admin_announcements_path
       expect(page).to have_content('Test Message')
@@ -33,10 +33,10 @@
     it 'updates the announcement message' do
       announcement = create(:announcement, message: 'Test Message', expires_at: 10.days.from_now)
       visit admin_announcement_path(announcement)
-      click_link('Edit')
+      click_on('Edit')
 
       fill_in :announcement_message, with: 'Edited test Message'
-      click_button('Save')
+      click_on('Save')
 
       visit admin_announcements_path
       expect(page).to have_content('Edited test Message')
@@ -59,7 +59,7 @@
       visit admin_announcement_path(announcement)
 
       accept_confirm do
-        click_link('Delete')
+        click_on('Delete')
       end
 
       expect(page).to have_current_path(admin_announcements_path)

spec/system/admin/deactivations_spec.rb

@@ -14,7 +14,7 @@
       find(:test_id, 'dropdown-button').click
 
       accept_confirm do
-        click_link('Deactivate')
+        click_on('Deactivate')
       end
     end
 
@@ -27,7 +27,9 @@
 
       fill_in 'Email', with: other_admin.email
       fill_in 'Password', with: other_admin.password
-      click_button 'Sign in'
+      within 'form' do
+        click_on 'Sign in'
+      end
 
       expect(page).to have_current_path(new_admin_user_session_path)
       expect(page).to have_content('Your account is deactivated')

spec/system/admin/flags_spec.rb

@@ -11,15 +11,15 @@
     visit admin_flags_path
 
     within("#flag_#{flag.id}") do
-      click_link('View')
+      click_on('View')
     end
   end
 
   context 'when dismissing the flag' do
     it 'dismisses the flag' do
-      click_button('Resolve flag')
+      click_on('Resolve flag')
       choose('action_taken_dismiss')
-      click_button('Submit')
+      click_on('Submit')
 
       expect(page).to have_content('Flag dismissed')
 
@@ -38,9 +38,9 @@
 
   context 'when removing the submission' do
     it 'removes the project submission' do
-      click_button('Resolve flag')
+      click_on('Resolve flag')
       choose('action_taken_removed_project_submission')
-      click_button('Submit')
+      click_on('Submit')
 
       expect(page).to have_content('Project submission removed')
 
@@ -69,9 +69,9 @@
 
   context 'when banning the submission owner' do
     it 'bans the project submission owner' do
-      click_button('Resolve flag')
+      click_on('Resolve flag')
       choose('action_taken_ban')
-      click_button('Submit')
+      click_on('Submit')
 
       expect(page).to have_content('Project submission owner has been banned')
 
@@ -111,9 +111,9 @@
 
   context 'when notifying the submission owner of a broken link' do
     it 'gives the admin feedback to let them know the user has been notified' do
-      click_button('Resolve flag')
+      click_on('Resolve flag')
       choose('action_taken_notified_user')
-      click_button('Submit')
+      click_on('Submit')
 
       expect(page).to have_content('Notification sent')
 

spec/system/admin/invitations_spec.rb

@@ -7,12 +7,12 @@
     # Create a new invitation
     visit admin_team_path
 
-    click_link('Invite new member')
+    click_on('Invite new member')
 
     fill_in('Name', with: 'John Doe')
     fill_in('Email', with: '[email protected]')
     select('Core', from: 'Role')
-    click_button('Send invite')
+    click_on('Send invite')
 
     expect(page).to have_content('Invitation sent to [email protected]')
     expect(page).to have_content('John Doe')
@@ -21,19 +21,19 @@
     using_session('john') do
       open_email('[email protected]')
       expect(current_email.subject).to match(/Joining The Odin Project Admin Team/)
-      current_email.click_link('Join the team')
+      current_email.click_on('Join the team')
 
       expect(page).to have_content('Welcome to the team!')
 
       find(:test_id, 'password-field').fill_in(with: 'supersecretpassword')
       find(:test_id, 'password-confirmation-field').fill_in(with: 'supersecretpassword')
-      click_button('Submit')
+      click_on('Submit')
 
       expect(page).to have_current_path(new_admin_two_factor_authentication_path)
 
       freeze_time do
         find(:test_id, 'otp-code-field').fill_in(with: otp_code_for(AdminUser.last))
-        click_button('Confirm')
+        click_on('Confirm')
       end
 
       expect(page).to have_current_path(admin_dashboard_path)
@@ -47,12 +47,12 @@
     # Create a new invitation
     visit admin_team_path
 
-    click_link('Invite new member')
+    click_on('Invite new member')
 
     fill_in('Name', with: 'John Doe')
     fill_in('Email', with: '[email protected]')
     select('Core', from: 'Role')
-    click_button('Send invite')
+    click_on('Send invite')
 
     expect(page).to have_content('Invitation sent to [email protected]')
     expect(page).to have_content('John Doe')
@@ -61,17 +61,17 @@
     using_session('john') do
       open_email('[email protected]')
       expect(current_email.subject).to match(/Joining The Odin Project Admin Team/)
-      current_email.click_link('Join the team')
+      current_email.click_on('Join the team')
 
       expect(page).to have_content('Welcome to the team!')
 
       find(:test_id, 'password-field').fill_in(with: 'supersecretpassword')
       find(:test_id, 'password-confirmation-field').fill_in(with: 'supersecretpassword')
-      click_button('Submit')
+      click_on('Submit')
 
       expect(page).to have_current_path(new_admin_two_factor_authentication_path)
 
-      click_link('Dashboard')
+      click_on('Dashboard')
       expect(page).to have_current_path(new_admin_two_factor_authentication_path)
       expect(page).to have_content('Please enable two factor authentication before continuing.')
     end

spec/system/admin/reactivations_spec.rb

@@ -14,7 +14,7 @@
       find(:test_id, 'dropdown-button').click
 
       accept_confirm do
-        click_link('Reactivate')
+        click_on('Reactivate')
       end
     end
 
@@ -25,17 +25,17 @@
     using_session('deactivated_admin') do
       open_email('[email protected]')
       expect(current_email.subject).to match(/Joining The Odin Project Admin Team/)
-      current_email.click_link('Join the team')
+      current_email.click_on('Join the team')
 
       find(:test_id, 'password-field').fill_in(with: 'supersecretpassword')
       find(:test_id, 'password-confirmation-field').fill_in(with: 'supersecretpassword')
-      click_button('Submit')
+      click_on('Submit')
 
       expect(page).to have_current_path(new_admin_two_factor_authentication_path)
 
       freeze_time do
         find(:test_id, 'otp-code-field').fill_in(with: otp_code_for(deactivated_admin.reload))
-        click_button('Confirm')
+        click_on('Confirm')
       end
 
       expect(page).to have_current_path(admin_dashboard_path)

spec/system/admin/sign_in_and_out_spec.rb

@@ -10,7 +10,9 @@
 
         fill_in 'Email', with: admin_user.email
         fill_in 'Password', with: admin_user.password
-        click_button 'Sign in'
+        within 'form' do
+          click_on 'Sign in'
+        end
 
         expect(page).to have_current_path(admin_dashboard_path)
       end
@@ -24,7 +26,9 @@
 
         fill_in 'Email', with: user.email
         fill_in 'Password', with: user.password
-        click_button 'Sign in'
+        within 'form' do
+          click_on 'Sign in'
+        end
 
         expect(page).to have_current_path(new_admin_user_session_path)
       end
@@ -38,11 +42,15 @@
 
         fill_in 'Email', with: admin_user.email
         fill_in 'Password', with: admin_user.password
-        click_button 'Sign in'
+        within 'form' do
+          click_on 'Sign in'
+        end
 
         expect(page).to have_content('Two factor authentication')
         fill_in 'Authentication code', with: otp_code_for(admin_user)
-        click_button 'Sign in'
+        within 'form' do
+          click_on 'Sign in'
+        end
 
         expect(page).to have_current_path(admin_dashboard_path)
       end
@@ -56,7 +64,9 @@
 
         fill_in 'Email', with: admin_user.email
         fill_in 'Password', with: admin_user.password
-        click_button 'Sign in'
+        within 'form' do
+          click_on 'Sign in'
+        end
 
         expect(page).to have_current_path(new_admin_user_session_path)
         expect(page).to have_content('Your account is deactivated')

spec/system/all_lesson_project_submissions_spec.rb

@@ -7,19 +7,19 @@
     let(:lesson) { create(:lesson, :project) }
 
     it 'paginates the results' do
-      create_list(:project_submission, 20, lesson:)
+      create_list(:project_submission, 20, lesson:) # rubocop:disable FactoryBot/ExcessiveCreateList
 
       sign_in(user)
       visit lesson_path(lesson)
-      click_link('View community solutions')
+      click_on('View community solutions')
 
       expect(page).to have_current_path(lesson_project_submissions_path(lesson))
 
       within(:test_id, 'submissions-list') do
         expect(page).to have_css('[data-test-id="submission-item"]', count: 15)
       end
 
-      click_link('Next')
+      click_on('Next')
 
       within(:test_id, 'submissions-list') do
         expect(page).to have_css('[data-test-id="submission-item"]', count: 5, visible: :all)
@@ -43,7 +43,7 @@
       sleep 0.4 # it will not open the dropdown without this
       find(:test_id, 'sort-select').trigger('click')
       expect(page).to have_link('Oldest')
-      click_link('Oldest')
+      click_on('Oldest')
 
       within(:test_id, 'submissions-list') do
         expect(page).to have_text(/oldest.+other.+newest/)

spec/system/lesson_project_submissions/add_submission_spec.rb

@@ -36,7 +36,7 @@
           expect(page).to have_no_content('Submit your solution')
         end
 
-        click_link('View community solutions')
+        click_on('View community solutions')
 
         within(:test_id, 'submissions-list') do
           page.driver.refresh

spec/system/user_notification_spec.rb

@@ -11,12 +11,12 @@
     visit admin_flags_path
 
     within("#flag_#{flag.id}") do
-      click_link('View')
+      click_on('View')
     end
 
-    click_button('Resolve flag')
+    click_on('Resolve flag')
     choose('action_taken_notified_user')
-    click_button('Submit')
+    click_on('Submit')
 
     sign_in(submission_owner)
   end

spec/system/user_settings_spec.rb

@@ -82,7 +82,7 @@
 
   describe 'changing password' do
     it 'validates password fields' do
-      click_link 'Password'
+      click_on 'Password'
 
       find(:test_id, 'current-password-field').fill_in(with: 'password')
       find(:test_id, 'password-field').fill_in(with: 'yo')