From 0e544e479e6dc3fc739434607ec794e1a5be4e2e Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Sat, 6 Jul 2024 16:08:01 -0600 Subject: [PATCH 01/10] Edit "Setup Credentials" step within workflows Addresses the following error that was encountered when attempting to execute the GitHub Actions that correspond with the edited workflows files: https://github.com/DMPRoadmap/roadmap/actions/runs/9513143079/job/26222602325 3s ``` Run bundle exec rails db:create RAILS_ENV=test To use retry middleware with Faraday v2.0+, install `faraday-retry` gem To use multipart middleware with Faraday v2.0+, install `faraday-multipart` gem; note: this is used by the ManageGHES client for uploading licenses Copying Bootstrap glyphicons to the public directory ... Copying TinyMCE skins to the public directory ... /home/runner/work/roadmap/roadmap/config/initializers/recaptcha.rb:8:in `block in
': undefined method `[]' for nil:NilClass (NoMethodError) from /home/runner/work/roadmap/roadmap/vendor/bundle/ruby/3.0.0/gems/recaptcha-5.17.0/lib/recaptcha.rb:37:in `configure' from /home/runner/work/roadmap/roadmap/config/initializers/recaptcha.rb:7:in `
' ``` --- .github/workflows/mysql.yml | 2 +- .github/workflows/postgres.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/mysql.yml b/.github/workflows/mysql.yml index eaf89f1b5c..9cb5d0d3cc 100644 --- a/.github/workflows/mysql.yml +++ b/.github/workflows/mysql.yml @@ -38,7 +38,7 @@ jobs: # Stub out the Rails credentials file so that we can start the Rails app - name: 'Setup Credentials' - run: EDITOR='echo "$(cat config/credentials.yml.mysql2)" >' bundle exec rails credentials:edit + run: EDITOR="sh -c 'echo \"$(cat config/credentials.yml.mysql2)\" > \$1' --" bundle exec rails credentials:edit # Set the path to the wkhtmltopdf executable - name: 'Determine wkhtmltopdf location' diff --git a/.github/workflows/postgres.yml b/.github/workflows/postgres.yml index d9c245a4e1..dc14fac17a 100644 --- a/.github/workflows/postgres.yml +++ b/.github/workflows/postgres.yml @@ -65,7 +65,7 @@ jobs: - name: 'Setup Credentials' run: | # generate a default credential file and key - EDITOR='echo "$(cat config/credentials.yml.postgresql)" >' bundle exec rails credentials:edit + EDITOR="sh -c 'echo \"$(cat config/credentials.yml.postgresql)\" > \$1' --" bundle exec rails credentials:edit # Set the path to the wkhtmltopdf executable - name: 'Determine wkhtmltopdf location' From 5801e7f4562e10c75d2bf2a5e9f7d594f3b6b30a Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Sat, 6 Jul 2024 16:53:19 -0600 Subject: [PATCH 02/10] Edit copy of credentials file within `bin/setup` Prior to this commit, the Rails credentials were not being updated during this setup process. --- bin/setup | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/setup b/bin/setup index 54cd0c40a1..19b9853e7b 100755 --- a/bin/setup +++ b/bin/setup @@ -30,7 +30,7 @@ if valid_db cp ".env.#{db_adapter}", '.env' puts "\n== Preparing credentials file ==" - system! "EDITOR='echo \"$(cat config/credentials.yml.#{db_adapter})\" >' bin/rails credentials:edit" + system!("EDITOR=\"sh -c 'echo \\\"$(cat config/credentials.yml.#{db_adapter})\\\" > \\\"\\\$1\\\"' --\" bin/rails credentials:edit") # Set the editor based on the platform ENV['EDITOR'] = Gem.win_platform? ? 'code --wait' : 'vim' From 727c2e861da46fb0d3703a4f4bef1727318ad0b4 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Mon, 8 Jul 2024 12:12:50 -0600 Subject: [PATCH 03/10] Disable Spring during workflows/ db build Here are links to the errors being raised prior to this commit: https://github.com/DMPRoadmap/roadmap/actions/runs/9822436610/job/27119190298?pr=3435 https://github.com/DMPRoadmap/roadmap/actions/runs/9822436613/job/27119190303?pr=3435 https://github.com/DMPRoadmap/roadmap/actions/runs/9844975903/job/27179509035?pr=3435 --- .github/workflows/mysql.yml | 4 ++-- .github/workflows/postgres.yml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/mysql.yml b/.github/workflows/mysql.yml index 9cb5d0d3cc..9ab6e6549f 100644 --- a/.github/workflows/mysql.yml +++ b/.github/workflows/mysql.yml @@ -54,8 +54,8 @@ jobs: - name: 'Build out the test database' run: | - bundle exec rails db:create RAILS_ENV=test - bundle exec rails db:schema:load RAILS_ENV=test + DISABLE_SPRING=1 bundle exec rails db:create RAILS_ENV=test + DISABLE_SPRING=1 bundle exec rails db:schema:load RAILS_ENV=test - name: 'Run any pending database migrations' run: bin/rails db:migrate RAILS_ENV=test diff --git a/.github/workflows/postgres.yml b/.github/workflows/postgres.yml index dc14fac17a..c1ee0d8cc8 100644 --- a/.github/workflows/postgres.yml +++ b/.github/workflows/postgres.yml @@ -79,7 +79,7 @@ jobs: # Initialize the DB - name: 'Setup Test DB' run: | - bundle exec rails db:setup RAILS_ENV=test + DISABLE_SPRING=1 bundle exec rails db:setup RAILS_ENV=test bundle exec rails db:migrate RAILS_ENV=test # Prebuild the CSS, JS and image assets From 1fe50c4a035687b7507cbabcb63868a4a8dda7ff Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Mon, 8 Jul 2024 12:52:04 -0600 Subject: [PATCH 04/10] Undo Rubocop `Style/SymbolProc`-related fixes This commit undoes some Rubocop fixes made from a prior commit ( bda5b6e95246ec9e54bc195d8bba8f933ec07487 ). However, it also resolves the following error that was being raised: https://github.com/DMPRoadmap/roadmap/actions/runs/9845084297/job/27179836711 --- app/models/org.rb | 2 +- app/models/template.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/org.rb b/app/models/org.rb index e17756a5f0..b56a40e0b1 100644 --- a/app/models/org.rb +++ b/app/models/org.rb @@ -141,7 +141,7 @@ class Org < ApplicationRecord before_validation :check_for_missing_logo_file # This gives all managed orgs api access whenever saved or updated. - before_save :ensure_api_access, if: lambda(&:managed?) + before_save :ensure_api_access, if: ->(org) { org.managed? } # If the physical logo file is no longer on disk we do not want it to prevent the # model from saving. This typically happens when you copy the database to another diff --git a/app/models/template.rb b/app/models/template.rb index 01805507bc..98de69ec8c 100644 --- a/app/models/template.rb +++ b/app/models/template.rb @@ -109,7 +109,7 @@ class Template < ApplicationRecord # overwriting the accessors. We want to ensure this template is published # before we remove the published_version # That being said, there's a potential race_condition where we have multiple-published-versions - after_update :reconcile_published, if: lambda(&:published?) + after_update :reconcile_published, if: ->(template) { template.published? } # ========== # = Scopes = From 283e5857e9441b29d289c2f8c4cf11341c978ca6 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Tue, 9 Jul 2024 14:19:55 -0600 Subject: [PATCH 05/10] Make Rubocop happy Omitting the arguments results in lambda implicitly using self, which appears to be the desired behaviour here. It also resolves the Rubocop offences. --- app/models/org.rb | 2 +- app/models/template.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/org.rb b/app/models/org.rb index b56a40e0b1..2f7aa76bb5 100644 --- a/app/models/org.rb +++ b/app/models/org.rb @@ -141,7 +141,7 @@ class Org < ApplicationRecord before_validation :check_for_missing_logo_file # This gives all managed orgs api access whenever saved or updated. - before_save :ensure_api_access, if: ->(org) { org.managed? } + before_save :ensure_api_access, if: -> { managed? } # If the physical logo file is no longer on disk we do not want it to prevent the # model from saving. This typically happens when you copy the database to another diff --git a/app/models/template.rb b/app/models/template.rb index 98de69ec8c..6cbb3c3e49 100644 --- a/app/models/template.rb +++ b/app/models/template.rb @@ -109,7 +109,7 @@ class Template < ApplicationRecord # overwriting the accessors. We want to ensure this template is published # before we remove the published_version # That being said, there's a potential race_condition where we have multiple-published-versions - after_update :reconcile_published, if: ->(template) { template.published? } + after_update :reconcile_published, if: -> { published? } # ========== # = Scopes = From 595e013d90e0c8436852b854c41cd65df527e13a Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Wed, 10 Jul 2024 14:09:38 -0600 Subject: [PATCH 06/10] Fix breaking template.visibilty tests `template.visibilty` now returns a string rather than an integer. The Rails 7 upgrade actually fixes a couple of bugs within `app/views/org_admin/templates/_form.html.erb` and `app/views/org_admin/templates/_show.html.erb`. Prior to this upgrade, template.visibility would return an integer. Now that it is returning a string, the `f.object.visibility == 'organisationally_visible'` and `template.visibility == 'organisationally_visible'` checks within the aforementioned files are behaving as desired. --- spec/models/template_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/models/template_spec.rb b/spec/models/template_spec.rb index dbb0a64a70..920a676955 100644 --- a/spec/models/template_spec.rb +++ b/spec/models/template_spec.rb @@ -1083,7 +1083,7 @@ end it 'sets visibility to Organisationally visible' do - expect(subject.visibility).to eql(Template.visibilities['organisationally_visible']) + expect(subject.visibility).to eql('organisationally_visible') end it 'sets is_default to false' do @@ -1152,7 +1152,7 @@ end it 'sets the visibility to Organisationally visible' do - expect(subject.visibility).to eql(Template.visibilities['organisationally_visible']) + expect(subject.visibility).to eql('organisationally_visible') end it 'sets is_default to false' do From 15e4d4df6eb82947c472ec476312a50f8e2fb859 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Wed, 10 Jul 2024 14:34:39 -0600 Subject: [PATCH 07/10] Refactor template.visibility w/ Rails enum method --- app/views/org_admin/templates/_form.html.erb | 2 +- app/views/org_admin/templates/_show.html.erb | 2 +- spec/models/template_spec.rb | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/views/org_admin/templates/_form.html.erb b/app/views/org_admin/templates/_form.html.erb index 0f2514b8cb..1d5e4d2161 100644 --- a/app/views/org_admin/templates/_form.html.erb +++ b/app/views/org_admin/templates/_form.html.erb @@ -20,7 +20,7 @@ placement: 'right' }%>
<%= f.label(:visibility) do %> - <%= f.check_box(:visibility, checked: f.object.visibility == 'organisationally_visible') %> + <%= f.check_box(:visibility, checked: f.object.organisationally_visible?) %> <%= _('for internal %{org_name} use only') % { org_name: f.object.org.name } %> <% end %> diff --git a/app/views/org_admin/templates/_show.html.erb b/app/views/org_admin/templates/_show.html.erb index 26a97cbd0d..f59c254872 100644 --- a/app/views/org_admin/templates/_show.html.erb +++ b/app/views/org_admin/templates/_show.html.erb @@ -31,7 +31,7 @@
<%= _('Visibility') %>
- <% if template.visibility == 'organisationally_visible' %> + <% if template.organisationally_visible? %> <%= _('for internal %{org_name} use only') % {org_name: template.org.name} %> <% else %> <%= _('available to the public') + (template.published? ? '' : ' (once published)') %> diff --git a/spec/models/template_spec.rb b/spec/models/template_spec.rb index 920a676955..018922efd5 100644 --- a/spec/models/template_spec.rb +++ b/spec/models/template_spec.rb @@ -1083,7 +1083,7 @@ end it 'sets visibility to Organisationally visible' do - expect(subject.visibility).to eql('organisationally_visible') + expect(subject.organisationally_visible?).to eql(true) end it 'sets is_default to false' do @@ -1152,7 +1152,7 @@ end it 'sets the visibility to Organisationally visible' do - expect(subject.visibility).to eql('organisationally_visible') + expect(subject.organisationally_visible?).to eql(true) end it 'sets is_default to false' do From 4bd0e9a5418b9c0596ab9dc9ab64f78f32d15248 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Wed, 10 Jul 2024 15:09:57 -0600 Subject: [PATCH 08/10] Fix `template.visibilty` checkbox behaviour Prior to this commit, the default checked/unchecked values were used (i.e. "1" would be returned when checked, and "0" would be returned when unchecked). However, the box is meant to be checked when selecting 'organisationally_visible' ('for internal %{org_name} use only'), which makes the default checked/unchecked values opposite to the mapping of our enums (i.e. `{"organisationally_visible"=>0, "publicly_visible"=>1}`). --- app/views/org_admin/templates/_form.html.erb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/views/org_admin/templates/_form.html.erb b/app/views/org_admin/templates/_form.html.erb index 1d5e4d2161..fbbf461466 100644 --- a/app/views/org_admin/templates/_form.html.erb +++ b/app/views/org_admin/templates/_form.html.erb @@ -20,7 +20,10 @@ placement: 'right' }%>
<%= f.label(:visibility) do %> - <%= f.check_box(:visibility, checked: f.object.organisationally_visible?) %> + <%= f.check_box(:visibility, + { checked: f.object.organisationally_visible? }, + f.object.class.visibilities[:organisationally_visible], + f.object.class.visibilities[:publicly_visible]) %> <%= _('for internal %{org_name} use only') % { org_name: f.object.org.name } %> <% end %> From d06f436e3c334304d0ccebb7703800ea46c36ec5 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Mon, 15 Jul 2024 14:11:52 -0600 Subject: [PATCH 09/10] Fix handling of JSON payload in API auth tests Rails 7 appears to apply stricter parsing rules. If the Content-Type is not JSON, then the body will not be parsed as JSON. --- spec/requests/api/v1/authentication_controller_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/requests/api/v1/authentication_controller_spec.rb b/spec/requests/api/v1/authentication_controller_spec.rb index e19fe6ab24..4f1b7b0a98 100644 --- a/spec/requests/api/v1/authentication_controller_spec.rb +++ b/spec/requests/api/v1/authentication_controller_spec.rb @@ -20,14 +20,14 @@ it 'calls the Api::Jwt::AuthenticationService' do Api::V1::Auth::Jwt::AuthenticationService.any_instance.expects(:call).at_most(1) - post api_v1_authenticate_path, params: @payload.to_json + post api_v1_authenticate_path, params: @payload, as: :json end it 'renders /api/v1/error template if authentication fails' do errs = [Faker::Lorem.sentence] Api::V1::Auth::Jwt::AuthenticationService.any_instance .stubs(:call).returns(nil) .stubs(:errors).returns(errs) - post api_v1_authenticate_path, params: @payload.to_json + post api_v1_authenticate_path, params: @payload, as: :json expect(response.code).to eql('401') expect(response).to render_template('api/v1/error') end @@ -35,7 +35,7 @@ token = Api::V1::Auth::Jwt::JsonWebToken.encode(payload: @payload) Api::V1::Auth::Jwt::AuthenticationService.any_instance.stubs(:call) .returns(token) - post api_v1_authenticate_path, params: @payload.to_json + post api_v1_authenticate_path, params: @payload, as: :json expect(response.code).to eql('200') expect(response).to render_template('api/v1/token') end From 5d06d6b86f6f8f6521e609b31e01ce5b4290f68a Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Thu, 18 Jul 2024 15:40:04 -0600 Subject: [PATCH 10/10] Fix `@rails/ujs` init / use ES6 module syntax Prior to this code change, any value assigned to the `'data-method':` attribute of the `link_to` method was not being read (and instead defaulting to `GET`). This was resulting in the breaking of several `spec/features/` tests (https://github.com/DMPRoadmap/roadmap/actions/runs/9946998559/job/27478725801). The `@rails/ujs` library is meant to handle this `'data-method':` attribute. --- app/javascript/application.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/app/javascript/application.js b/app/javascript/application.js index ccad6aa6a5..4df7f76917 100644 --- a/app/javascript/application.js +++ b/app/javascript/application.js @@ -81,10 +81,11 @@ import './src/superAdmin/notifications/edit'; import './src/superAdmin/themes/newEdit'; import './src/superAdmin/users/edit'; -// Since we're using Webpacker to manage JS we need to startup Rails' Unobtrusive JS -// and Turbo. ActiveStorage and ActionCable would also need to be in here -// if we decide to implement either before Rails 6 -require('@rails/ujs').start(); +// We need to startup Rails' Unobtrusive JS and Turbo. +// ActiveStorage and ActionCable would also need +// to be in here if we decide to implement either. +import Rails from "@rails/ujs"; +Rails.start(); // TODO: Disabled turbo for the time being because our custom JS is not // properly setup to work with it. We should review the docs: