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

Fix time ago when making a notification read/unread. #6821

Closed
wants to merge 1 commit into from
Closed

Fix time ago when making a notification read/unread. #6821

wants to merge 1 commit into from

Conversation

ralinc
Copy link
Contributor

@ralinc ralinc commented May 5, 2016

Marking a notification as unread resets the timeago stamp causing
the times to look wrong. It can be reproduced by marking an old
notification as unread. Using the update_colum instead of
update_attribute will not touch the updated_at attribute, and
thus will not affect the updated time ago in the view.

Fixes #6798.

@@ -41,7 +41,11 @@ def email_the_user(target, actor)
end

def set_read_state( read_state )
self.update_attributes( :unread => !read_state )
if self.new_record?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is only called on existing notifications, so this if shouldn't be needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The if-statement is needed because of some failing model unit tests. Or we have to change the tests somehow.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't change the code only to make tests running. Please change the tests that they use saved notifications, so the tests correspond to the real conditions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course. I agree.

Marking a notification as unread resets the timeago stamp causing
the times to look wrong. It can be reproduced by marking an old
notification as unread. Using the update_column instead of
update_attribute will not touch the updated_at attribute, and
thus will not affect the updated time ago in the view.

Fixes #6798.
@denschub denschub closed this in 1773e3e May 7, 2016
@denschub
Copy link
Member

denschub commented May 7, 2016

Merged as 1773e3e, thank you for your contribution!

@denschub denschub added this to the 0.5.9.0 milestone May 7, 2016
@ralinc ralinc deleted the fix-time-ago-in-notifications branch May 7, 2016 17:00
@Flaburgan
Copy link
Member

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants