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

Upgrades to Rails 6 #470

Merged
merged 23 commits into from
Jul 9, 2020
Merged

Upgrades to Rails 6 #470

merged 23 commits into from
Jul 9, 2020

Conversation

atruskie
Copy link
Member

@atruskie atruskie commented Jul 6, 2020

Includes updates to all dependencies plus many lint and spec fixes

Also removes several old dependencies.
Revent version bump means new rules were added and some old rules were removed
Folds in resque-status gem because source code base is unmaintained.

Adds sprockets manifest file.

Enables reloading for web app if running development server.
- move all autoload code into app where rails will be able to load it without issue or configuration
- move gem folders into proper gem structures
- cleaned up application.rb - moving more things to  initializers makes it much clearer what is going on
- started  to deprecate settings logic (which is unmaintained)
- removed trim from nameywamey - it's better suited as a patch that everything can use
SettingsLogic is no longer maintained, so I've switched to rubyconfig/config.

Also cleared out as many warnings as I could during app boot.

This means  that autoloading constants is no longer allowed in initializers. Changes were made to including AlphabeticalPaginatorQuery and the i8ln-inflector railtie (see new patch).

WIP: tests still need to be run
Tested with rails zeitwerk:check
Also added support for use database_cleaner on redis
Main thing is that belongs_to associations are now required by default. Also removed old validators.
Mainly involved adding :optional to associations that were optional (since associations are now required by default in rail 6+)

Also had to override more of the big decimal json serialization so it would actually take effect when serializing notes to the database.

Defined helper methods to refer to the default dataset as well and ensured the helpers were used in the seeds and tests.

-> { uniq}  is deprecated and needed to be replaces with distinct.

the activesupport parameterize method requires named arguments to work now.
Partially adresses #467.

The now deprecated `serialize` constrtuct that used to be applied to models that store hashes in a text field is *now* taken into account when AREL constructs queries. The result was a double-encoded JSON string used a value to search for in the database which would obviously never match anything!

Made the following changes to adress this:
- simplified strong params
- migrated stored_query to jsonb
- allowed our API code to accept a Hash as a primitive value to compare against in queries, IFF the type of the column of the attribute is JSON.
- and adjusted the acceptance test so that a true hash is passed rather than a JSON encoded string
Also added patches to AREL for retrieving the type of an attribute.

Also canonicalised our is_json postgres function.
After the rails 6 upgrade the container no longer booted correctly.
- bumped our bundler version
- attempted to run uby 2.7 but our dependecies do not allow it (nokogiri being the main offender)
- Ensured database configuration works in multi-database mode. By default db: commands now operate on development and test environments at the same time (a rails default).
- simplified the app boot experience by:
  - wholly incorporating the migrate experience into the rake task
  - which removed the need for  two bash files migrate.sh and dev_setup.sh (along with multi-database feature)
- fixed an issue where local gems were not avaiable for bundle install because they were not yet copied to the container
- fixed an issue where workers would not boot because dry-validation was not loaded
- moved the cors code to an initializer file
Now ensures workers cannot access postgres db by placing them on serparate networks.
Also corrects condition on which datbase migration checks are run.

Also wrapped database keys in config in strings - seems to obviate the rails config/erb-simplify warning.
The .from method was changed in rails/arel@98fc259

Fixes the method and relvant tests (along with formatting).

Fixes:      ArgumentError:
       wrong number of arguments (given 1, expected 0)
     # /usr/local/bundle/gems/activerecord-6.0.3.2/lib/arel/table.rb:33:in `from'
- mainly strong params fixes
- strong params is no longer a hash so also a fair few calls to to_h were added to coerce params into a hash that the rest of our lib could deal with it
- was also able to simplify some hash params filtering (re #467 )
- fixed error handling due to error being moved from env to request.env for uncaught error
- adds support for type casting columns in arel
- that is used to allow us to use `contains` filters on json columns (subet.rb)
File should have been committed with a02c880
Something changed somewhere that changed wether or not AREL outputs parentheses around an expression. In this case the change was sginficant because the RHS of the permissions expression short-circuited the rest of the predicates

Used a Arel::Nodes::Grouping to enforce parantheses output.
- completely removed BawWorkers::Settings alias. It introduced subtle bugs for the sake of avoiding a mass change - the mass change ended up being easier
- removed use_ssl as an option for worker API access. We're pretty reliant on https now so there's no use having an option that disables ssl
- conveted workers mailer to use erb views - ended up being simpler than rendering inline content since rails requires views must exist regardless
@atruskie atruskie requested a review from Allcharles July 6, 2020 12:55
@atruskie atruskie mentioned this pull request Jul 6, 2020
@Allcharles Allcharles added architectural dependencies Pull requests that update a dependency file enhancement and removed architectural labels Jul 8, 2020
The method used to sanitize hashes recieved by the API was no longer viable in Rails 6. I had to adapt the method and in doing so I also wrote more tests.

Additionally, the factory bot code somehow imported the database status rake task and was using methods from that file when constructing factories. Not only should this be impossible but it is also very bad. I renamed the methods in the rake file so at a minimum they could not be mistakenly referenced (due to sharing common identifiers ). Root cause unknown.
It was using hard coded unique ids that failed due to a primary key conflict. Replaced with a dyanmic expression.
@atruskie atruskie merged commit 6548df0 into master Jul 9, 2020
@atruskie atruskie deleted the rails-6 branch July 9, 2020 00:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants