refactor: remove RAILS_MASTER_KEY from CI & upgrade to rails 7.1

[guidojw]

Checklist

• Merged database migrations into 1 database migration.
• Tested database migrations from origin/staging (git checkout staging ; git pull ; bundle exec rails db:reset ; git checkout BRANCH ; bundle exec rails db:migrate).

[lodewiges]

is this ready to merge?, if not what should be changed and can i do this?

[guidojw]

is this ready to merge?, if not what should be changed and can i do this?

It is not, I'll update this comment with what's to be done later

[guidojw]

@lodewiges it's finished now, the goal was to get RAILS_MASTER_KEY out of the ci workflow so that PRs from external forks would be able to work, but the Mollie tests call the real Mollie API and need a key for that so I compromised a little bit and kept it for the tests

Edit: it is possible to get secrets on PRs from forks, but it's kind of a hack https://michaelheap.com/access-secrets-from-forks/

[lodewiges]

Thank you for your fixing it Guido, I understand why we still need the rails key, However should it be possible to give Mollie a fake api key when the environment is test. this requires a bit of logic in aplication.rb. But it should solve the problem I think.

[guidojw]

With a fake api key how would the tests work? It's a real api it calls which will throw unauthorised errors then

[guidojw]

The same compromise has to be made on amber-api, there the ImprovMX api is called in the tests.
Making these few test conditional based on if the master key exists (not in fork prs then) is something I'm considering now

[lodewiges]

The same compromise has to be made on amber-api, there the ImprovMX api is called in the tests. Making these few test conditional based on if the master key exists (not in fork prs then) is something I'm considering now

I thought we only needed a key to initialize mollie. I was not aware that we needed to used the production key

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.17%. Comparing base (5c1018a) to head (6cac46a).
Report is 5 commits behind head on staging.

Additional details and impacted files

@@           Coverage Diff            @@
##           staging     #890   +/-   ##
========================================
  Coverage    75.17%   75.17%           
========================================
  Files           55       55           
  Lines         1116     1116           
========================================
  Hits           839      839           
  Misses         277      277           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[lodewiges]

Code looks good, I will not approve yet because we need to upgraded some deprecated things before moving to rails 7.1