Defect #6549
closed
Notification settings not migrated properly
Added by Felix Schäfer over 13 years ago. Updated over 13 years ago.
0%
Description
The migration in r4216 to update users' notification settings from boolean to string-format doesn't correctly migrate the "For any event on the selected projects only…" (the new :selected option) option, those users get "updated" to "Only for things I watch or I'm involved in"/:only_my_events.
Related issues
Updated by Felix Schäfer over 13 years ago
Ok, so a follow-up on this: as I said earlier, users who had selected "For any event on the selected projects only…" get updated to "Only for things I watch or I'm involved in", which isn't a desired state. The hotfix for people who have been bitten by this: open a console (run RAILS_ENV=production script/console in your redmine directory) and execute the following command:
User.find_all_by_mail_notification('only_my_events').select {|u| !u.notified_projects_ids.empty?}.each {|u| u.mail_notification = 'selected'; u.save!}2 things: this will only work for people whose account hasn't been "written to" since the faulty migration, this won't work for users who have been to their account page and changed something, be it the mail notification options or their name or whatever. The hotfix only works because of a dangling "data inconsistency" left over by the faulty migration and corrects that, users who have changed their account in some way have overwritten this. The second thing is: I take no responsibility whatsoever in what happens with your data with the above line, it worked well on my test and production rig, if it blows your DB, well, you should replay this backup you made beforehand.
So much for the hotfix, I have a probable fix for the migration, which I have to test a little bit more before posting it here.
(oh, and if you want a list of affected users, this should give you the list of mail addresses of those users, same conditions as above apply:
User.find_all_by_mail_notification('only_my_events').select {|u| !u.notified_projects_ids.empty?}.collect(&:mail)Replace the mail in the collect(&:mail) at the end with another attribute to get a list of this attribute of the affect users.)
Updated by Felix Schäfer over 13 years ago
- Status changed from New to 7
Ok, here's the commit to fix the migration: http://github.com/thegcat/redmine/commit/811bc0863015c2d2ed87f71edaec95560ada9318
Here's the diff of the proposed change, comments after that:
diff --git a/db/migrate/20100129193813_update_mail_notification_values.rb b/db/migrate/20100129193813_update_mail_notification_values.rb
index bfe0a59..4b49d87 100644
--- a/db/migrate/20100129193813_update_mail_notification_values.rb
+++ b/db/migrate/20100129193813_update_mail_notification_values.rb
@@ -1,8 +1,12 @@
# Patch the data from a boolean change.
class UpdateMailNotificationValues < ActiveRecord::Migration
def self.up
- User.update_all("mail_notification = 'all'", "mail_notification = '1'")
- User.update_all("mail_notification = 'only_my_events'", "mail_notification = '0'")
+ User.record_timestamps = false
+ User.all.each do |u|
+ u.mail_notification = (u.mail_notification =~ /\A(1|t)\z/ ? 'all' : (u.notified_projects_ids.empty? ? 'only_my_events' : 'selected'))
+ u.save!
+ end
+ User.record_timestamps = true
end
def self.downI didn't find any "nice" way to get this done all through User.update_all, so I went the programmatic way. The timestamps are disabled for the duration of the change so that all users don't their updated_on bumped up needlessly to the time the migration is applied. This migration also takes into account that some databases migrate a true boolean field to t and not to 1 when the field is changed to a varchar.
Eric: Want me to merge that with the other patch from #6541, or will you take care of it?
Updated by Felix Schäfer over 13 years ago
Oddly this didnt fix my Trunk SVN Redmine site
Chris, this doesn't fix already migrated sites, i.e. if you already applied the migration when there were the User.update_all blurbs in there, it won't fix anything, the patch I submitted to github only prevents this error from happening for people who will run the migration later on.
I suspect you had already run the faulty migration, please see my first comment on this page for a way to correct this.
Updated by Chris McKee over 13 years ago
What modifications this making to the users table (wondering as the first script wont run in WinServer2k3)
Would I be better off restoring the user preferences table from pre-upgrade?
Updated by Chris McKee over 13 years ago
I should add I'm using MySQL and mail notification is being set to 1 for groups while users obviously still have their own settings only_my_events.
Not so much fun in a top500 company when your VP starts to get updates on web tasks :|
Updated by Felix Schäfer over 13 years ago
Chris McKee wrote:
Not so much fun in a top500 company when your VP starts to get updates on web tasks :|
Well, maybe you shouldn't run trunk then ;-) Repeat after me: trunk is not stable, trunk is not stable, trunk…
Anyway, the line in comment 1 updates the settings correctly, but doesn't solve the problem which causes mails to be sent to every project member rather than just to those who requested it, see #6541 for more info and a patch for that.
Updated by Chris McKee over 13 years ago
Felix Schäfer wrote:
Well, maybe you shouldn't run trunk then ;-) Repeat after me: trunk is not stable, trunk is not stable, trunk…
To be fair id only updated to the 1.0.2 tag; the errors even in the Bitnami release + tag
Updated by Felix Schäfer over 13 years ago
Chris McKee wrote:
Felix Schäfer wrote:
Well, maybe you shouldn't run trunk then ;-) Repeat after me: trunk is not stable, trunk is not stable, trunk…
To be fair id only updated to the 1.0.2 tag; the errors even in the Bitnami release + tag
Then you shouldn't have this migration in yet at all, this is staged for 1.1 at the earliest…
Updated by Chris McKee over 13 years ago
Curse of automation, I think our trunk server ended up updating the main servers email settings so the database migration took place for dev into live. What a difference an xcopy makes.
Ah well even with mass mailing its still more usable than Sharepoint.
Updated by Chris McKee over 13 years ago
Adding this to a clean 'trunk' and manually rolling back the user table and running #6541 in script/console seems to have fixed it :o)
Updated by Eric Davis over 13 years ago
- Status changed from 7 to Closed
- Target version set to 1.1.0
- Resolution set to Fixed
Fixed in r4246 with a modified version of Felix's patch.
Updated by Andreas R over 13 years ago
I just updated to my svn instalation from svn trunk (Revision: 4425 and read the above comments about using trunk) and found a funny thing:
My database didn't migrate the mail_notification from bool to string but db/migrate/20100129193402_change_users_mail_notification_to_string.rb was there and looked good.
Made the migrations manually and the it worked.
Is there any reason while a migration didn't get executed? Can i look somehwere what went wrong? Nothing n log/production.log though.