Conversation
This is to ensure that applications running migrations and seeding in the same process (via `rails db:setup`, for example), don't get column definition errors because models with not yet fully populated schemas were loaded too early and stay in memory until the migrations are run.
So that we can detect incomplete models loaded during seeds.
e949f0c to
f7bed24
Compare
Codecov Report
@@ Coverage Diff @@
## master #2933 +/- ##
=======================================
Coverage 98.72% 98.72%
=======================================
Files 1656 1656
Lines 39458 39458
=======================================
Hits 38956 38956
Misses 502 502 |
| .where(deleted_at: nil) | ||
| .where(managed: false) | ||
| .find_each { |u| u.update(nickname: Decidim::User.nicknamize(u.name)) } | ||
| User.where(nickname: nil) |
There was a problem hiding this comment.
How does this solve the issue? Is it something about namespaces and class reloading? 😕
Won't this clash if the app, for some reason, has a User model? I'm thinking maybe there's a way to force Rails to load all classes before running migrations...
There was a problem hiding this comment.
This is a technique we've been using for a while. You try to use plain SQL but if you need AR, you use a dummy model scoped to the migration. In this case, I'm loading the class FixNicknameIndex::User. The idea is that if I load the real class now, that version will stay loaded, and that version is incomplete because we're in the middle of migrations... So we don't use the real classes during migrations.
There was a problem hiding this comment.
Oh, I didn't see the class is defined on top 😅 (GitHub didn't render that part)
👍
| # db:seed`), we make sure to run them in the same process if seeds are | ||
| # requested so that we can catch these situations earlier than end | ||
| # users. | ||
| if options[:seed_db] |
There was a problem hiding this comment.
I actually splitted this some time ago because seeding was erroring when running in the same process as migrations 🤨
There was a problem hiding this comment.
Yes, now it's not. Maybe the solution is to never run that command but I find it handy to seed and migrate in one command... 😔
|
Just a doubt regarding this issue: this bug wasn't detected by CI because we're not testing the flow of "installing decidim gem, creating an application, creating the DB, migrating, seeding, starting rails server and checking for a 200 HTTP status code on http://localhost:3000", right? |
|
@andreslucena We're not doing that indeed, and we should. Right now we only check the "installing decidim gem, creating an application, creating the DB, migrating, seeding" part, but we don't boot a rails server on the generated app and check that the homepage loads. I tried to implement that once and failed, and came to the conclusion that the best way to test that would be to automatically configure RSpec in the generated application and create a dummy system spec that loads the homepage. And run that. However, this issue is not directly related and only affects generated applications tangentially. Generated applications are currently generated and seeded fine. This problem would be hit only if you generate and seed an application, work on it for a bit and at somepoint you want it back to the initial DB state. Then depending on the command you use, it could fail: This PR just fixes migrations so that none of the above commands fail. |
|
Also note that we're recommending the failing command in our docs. I actually found out about these kind of issues for the first time because of a gitter report. |
* master: [RFC] Use cells for meeting m cards (#3022) Do not force Postgresql user to be admin when enabling trigram extension (#3053) Make organization reference_prefix required (#3056) admin can duplicate/copy meetings (#3051) Fix question form errors not being displayed (#3046) Erb whitespace cutting (#3047) Show debates statistics on space show and homepage (#3016) Fix broken translated field after form errors (#3026) Move decidim executable to "exe" folder (#3028) Friendlier buttons (#3027) Feedback needed after Endorsing when user has no user_groups (#2998) Fix seeding error on generator specs (#3021) fix spelling error in threshold (#3019) Migration plus seeds (#2933)
🎩 What? Why?
I got the following error when running migrations and seeding my app.
📌 Related Issues
None.
📋 Subtasks
None.
📷 Screenshots (optional)
None.