Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to Rails 7 and mongoid 8 #217

Merged
merged 9 commits into from
Mar 7, 2024
Merged

Upgrade to Rails 7 and mongoid 8 #217

merged 9 commits into from
Mar 7, 2024

Conversation

jkotanchik-SB
Copy link
Contributor

@jkotanchik-SB jkotanchik-SB commented Feb 22, 2024

The MADiE Team is working towards using cqm-reports to generate QRDA exports and need to update cqm-models to resolve vulnerabilities in rails 5 and nokogiri.

I opted to move rails up as far as I could to avoid having to do this dance again anytime soon. That resulted in Rails 7 and Mongoid 8, as well as the below changes to resolves unit test failures. I'm not deeply familiar with this codebase, so any feedback on changes made would be appreciated.

If moving to Rails 6 would be preferred, that is an option. I did test with Rails 6 and Mongoid 7, and though I had to make the same changes, it did result in a clean report from bundle audit.

If preferred, we can skip merging to master and maintain these changes on their own branch. We also considered releasing these changes as new minor version to the ruby gem.

Submitter:

  • This pull request describes why these changes were made.
  • Internal ticket for this PR:
  • Internal ticket links to this PR
  • Code diff has been done and been reviewed
  • Tests are included and test edge cases
  • Tests have been run locally and pass
  • If applicable, the library version number in package.json and cqm-models.gemspec has been updated
  • Cqm-execution fixtures have been updated with the update_cqm_execution_fixtures.sh script inside server-scripts using this branch in the cqm-converter

Bonnie Reviewer:

Name:

  • Code is maintainable and reusable, reuses existing code and infrastructure where appropriate, and accomplishes the task’s purpose
  • The tests appropriately test the new code, including edge cases
  • You have tried to break the code

Cypress Reviewer:

Name:

  • Code is maintainable and reusable, reuses existing code and infrastructure where appropriate, and accomplishes the task’s purpose
  • The tests appropriately test the new code, including edge cases
  • You have tried to break the code

 Resolves unit test failure as a workaround for updates in mongoid 8 (and 7) that expect uniqueness checks against explicit fields.
@jkotanchik-SB jkotanchik-SB self-assigned this Feb 22, 2024
@dczulada dczulada self-requested a review February 26, 2024 16:20
@dczulada
Copy link
Contributor

@jkotanchik-SB I will review this PR.

On Cypress we have been using this tagged version of cqm-models (https://github.com/projecttacoma/cqm-models/tree/cypress_v7.0.0) because we wanted to support using QDM v5.5 and v5.6.

On Cypress we have been using it with Ruby v3.2 (currently 3.2.2, Rails 7 (currently 7.0.7.2), Mongoid 7 (currently 7.5.2), nokogiri 1.14.5.

We are also planning on moving to monoid v8 too.

Getting these changes into the master branch will be nice.

* additional mongoid updates

* remove check for validate_generator
…ement base type, making this `if` branch duplicative.
@jkotanchik-SB
Copy link
Contributor Author

@dczulada Just to be sure you saw, I bumped the ruby version from 4.1.X to 4.2.0. I did not change the version in package.json.

Does that change make sense for your workflows? And would you like me to bump up the package.json version as well?

@dczulada
Copy link
Contributor

dczulada commented Mar 6, 2024

@dczulada Just to be sure you saw, I bumped the ruby version from 4.1.X to 4.2.0. I did not change the version in package.json.

Does that change make sense for your workflows? And would you like me to bump up the package.json version as well?

The PR here does bump the version to 4.2.0 in the gemspec. I think that version increment makes sense to me.

@jkotanchik-SB jkotanchik-SB merged commit b8fd3ef into master Mar 7, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants