add migrations for upcoming activities for commands/profiles #17472
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feat-ua-commands #17472 +/- ##
====================================================
+ Coverage 65.58% 65.60% +0.01%
====================================================
Files 1193 1194 +1
Lines 107927 108014 +87
Branches 2568 2568
====================================================
+ Hits 70788 70865 +77
- Misses 31759 31767 +8
- Partials 5380 5382 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| INSERT INTO user_persistent_info (user_id, user_name) | ||
| SELECT id, name | ||
| FROM users | ||
| `) |
There was a problem hiding this comment.
two questions:
- should we drop
users.nameand force people toJOINusing this table? - if we don't do
1should we use a db trigger? I know we avoid those but I don't remember why (maybe performance for thehoststable?)
There was a problem hiding this comment.
Good questions... Would it be very hard to avoid both? I.e. keep the mirrored info (as we also do for activities I think?), and avoid the trigger by making sure the updates of users also update that table in a transaction?
There was a problem hiding this comment.
sounds good to me! ended up going with that.
roperzh
changed the title
| user_id int(10) unsigned DEFAULT NULL, | ||
|
|
||
| -- user_name mirrors the users.name value | ||
| user_name varchar(255) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT '', |
There was a problem hiding this comment.
Should we also mirror the gravatar_url? On second thought probably not, I see the activities table doesn't have it (thought it did for some reason).
There was a problem hiding this comment.
hm, but I think we compute the gravatar from the email, I will sanity check and include the email as well.
| INSERT INTO user_persistent_info (user_id, user_name) | ||
| SELECT id, name | ||
| FROM users | ||
| `) |
There was a problem hiding this comment.
Good questions... Would it be very hard to avoid both? I.e. keep the mirrored info (as we also do for activities I think?), and avoid the trigger by making sure the updates of users also update that table in a transaction?
| ALTER TABLE`+" `%s` "+` | ||
| ADD CONSTRAINT`+" `fk_%s_user_info` "+` | ||
| FOREIGN KEY (user_persistent_info_id) REFERENCES user_persistent_info(id) | ||
| ON DELETE RESTRICT`, t, t)) |
There was a problem hiding this comment.
DELETE RESTRICT is basically to prevent a deletion to user_persistent_info, which should never happen, right?
There was a problem hiding this comment.
that's exactly right! just as an extra precaution.
#16830
Checklist for submitter
If some of the following don't apply, delete the relevant line.