roadmap icon indicating copy to clipboard operation
roadmap copied to clipboard

Fix PostgreSQL and MySQL2 GitHub Actions For Rails 7 Upgrade

Open aaronskiba opened this issue 1 year ago • 3 comments

Changes proposed in this PR:

  • This PR seeks to fix the PostgreSQL and MySQL2 GitHub actions for PR #3426
    • The copying of credentials via the config/credentials.yml.mysql2 config/credentials.yml.postgresql had to be updated for the Rails 7 upgrade.
    • DISABLE_SPRING=1 has been added to the test db setup commands. Without disabling Spring, the test db was failing to build with the Rails 7 upgrade.
    • The Style/SymbolProc-related Rubocop fixes applied in commit bda5b6e95 have been undone. Although executing rubocop -A results in these changes, those changes were causing errors within the code.

aaronskiba avatar Jul 06 '24 22:07 aaronskiba

</tr>
1 Error
:no_entry_sign:

Please include a CHANGELOG entry.

You can find it at [CHANGELOG.md](https://github.com/DMPRoadmap/roadmap/blob/main/CHANGELOG.md).

Generated by :no_entry_sign: Danger

github-actions[bot] avatar Jul 08 '24 18:07 github-actions[bot]

The failing tests still need to be addressed for the PostgreSQL GitHub action, but the workflow is now executing.

The MySQL GitHub action is still failing to execute. Many errors like the following are being thrown:

ActiveRecord::MismatchedForeignKey: Column `org_id` on table `annotations` does not match column `id` on `orgs`, which has type `bigint unsigned`. To resolve this issue, change the type of the `org_id` column on `annotations` to be :bigint. (For example `t.bigint :org_id`).

These errors are related to the migrations added to https://github.com/DMPRoadmap/roadmap/pull/3426. When the migrations were run, many tables not targeted by the migrations were also updated. Here is an example:

# before running migrations
create_table "orgs", id: :integer, force: :cascade do |t|
# after running migrations
create_table "orgs", id: :serial, force: :cascade do |t|

It seems that :serial is interpreted differently by MySQL and PostgreSQL. I think PostgreSQL casts it to type :integer, whereas MySQL casts it to type :bigint. Because the associated foreign keys are defined as type :integer, This would explain why the MySQL GitHub Action is now failing.

I'm not sure how to proceed here. Should we simply retire the MySQL GitHub Actions? Or perhaps re-running the migrations with a MySQL db would re-cast the primary keys to type :integer (I can't seem to get ruby bin/setup mysql to work on my machine)?

Wondering what your thoughts might be here @benjaminfaure and @briri. Thank you.

aaronskiba avatar Jul 09 '24 16:07 aaronskiba

There are now two failing tests within the PostgreSQL workflow. Although the tests were passing prior to this Rails 7 upgrade, there were known issues prior to it:

  • https://github.com/DMPRoadmap/roadmap/issues/3419
  • https://github.com/DMPRoadmap/roadmap/pull/3420
Failures:

  1) Template#customize! sets visibility to Organisationally visible
     Failure/Error: expect(subject.visibility).to eql(Template.visibilities['organisationally_visible'])

       expected: 0
            got: "organisationally_visible"

       (compared using eql?)
     # ./spec/models/template_spec.rb:1086:in `block (3 levels) in <main>'

  2) Template#upgrade_customization! sets the visibility to Organisationally visible
     Failure/Error: expect(subject.visibility).to eql(Template.visibilities['organisationally_visible'])

       expected: 0
            got: "organisationally_visible"

       (compared using eql?)
     # ./spec/models/template_spec.rb:1155:in `block (3 levels) in <main>'

aaronskiba avatar Jul 09 '24 21:07 aaronskiba

There are now two failing tests within the PostgreSQL workflow. Although the tests were passing prior to this Rails 7 upgrade, there were known issues prior to it:

The aforementioned issues have been addressed, but now the following ./spec/requests/api/v1/authentication_controller_spec.rb tests are breaking:

Failures:
  1) Api::V1::AuthenticationController actions POST /api/v1/authenticate renders /api/v1/error template if authentication fails
     Failure/Error: expect(response.code).to eql('401')
       expected: #<Encoding:UTF-8> "401"
            got: #<Encoding:US-ASCII> "400"
       (compared using eql?)
     # ./spec/requests/api/v1/authentication_controller_spec.rb:31:in `block (4 levels) in <main>'
  2) Api::V1::AuthenticationController actions POST /api/v1/authenticate returns a JSON Web Token
     Failure/Error: expect(response.code).to eql('[200](https://github.com/DMPRoadmap/roadmap/actions/runs/9881513683/job/27292456367?pr=3435#step:13:201)')
       expected: #<Encoding:UTF-8> "200"
            got: #<Encoding:US-ASCII> "400"
       (compared using eql?)
     # ./spec/requests/api/v1/authentication_controller_spec.rb:39:in `block (4 levels) in <main>'

aaronskiba avatar Jul 10 '24 21:07 aaronskiba

Failures:
  1) Api::V1::AuthenticationController actions POST /api/v1/authenticate renders /api/v1/error template if authentication fails
     Failure/Error: expect(response.code).to eql('401')
       expected: #<Encoding:UTF-8> "401"
            got: #<Encoding:US-ASCII> "400"
       (compared using eql?)
     # ./spec/requests/api/v1/authentication_controller_spec.rb:31:in `block (4 levels) in <main>'
  2) Api::V1::AuthenticationController actions POST /api/v1/authenticate returns a JSON Web Token
     Failure/Error: expect(response.code).to eql('[200](https://github.com/DMPRoadmap/roadmap/actions/runs/9881513683/job/27292456367?pr=3435#step:13:201)')
       expected: #<Encoding:UTF-8> "200"
            got: #<Encoding:US-ASCII> "400"
       (compared using eql?)
     # ./spec/requests/api/v1/authentication_controller_spec.rb:39:in `block (4 levels) in <main>'
   29:       # POST /api/v1/authenticate
   30:       # rubocop:disable Metrics/AbcSize
   31:       def authenticate
   32:         byebug
=> 33:         body = request.body.read
   34:         json = JSON.parse(body)
   35:         auth_svc = Api::V1::Auth::Jwt::AuthenticationService.new(json: json)
   36:         @token = auth_svc.call
   37: 
(byebug) request.headers['Content-Type']
"application/x-www-form-urlencoded"
(byebug) request.body.read
""

Rails 7 appears to apply stricter parsing rules. If the Content-Type is application/x-www-form-urlencoded, the body will not be parsed as JSON.

The following code changes throughout the codebase seem to resolve this:

# Old Code
post api_v1_authenticate_path, params: @payload.to_json
# New Code (use `as:` for encoding the request's content type)
post api_v1_authenticate_path, params: @payload, as: :json

aaronskiba avatar Jul 12 '24 18:07 aaronskiba

Hi @benjaminfaure, thanks for the approval on this PR. I added a couple more commits, so maybe it'd be best to get one more review/approval before I merge it into your Rails 7 upgrade branch? Thank you. :)

aaronskiba avatar Jul 22 '24 14:07 aaronskiba