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

Mark unread submissions #1443

Merged
merged 5 commits into from
Nov 15, 2016
Merged

Mark unread submissions #1443

merged 5 commits into from
Nov 15, 2016

Conversation

redshiftzero
Copy link
Contributor

This PR is to merge in the work of @mdrose from #1358 which adds in an unread icon on the submissions view for submissions that are unread as well as the option to download all unread messages from the submission view. I broke out the getting unread submissions functionality and added a couple unit tests for it. Let me know if other tests need writing and I will add! Closes #1353.

Copy link
Contributor

@psivesely psivesely left a comment

Choose a reason for hiding this comment

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

  1. I don't think we need the extra Python functions to accomplish this webapp functionality. Why not just use the doc.downloaded attribute? (As a side note, I'm not a fan of how inconsistent naming is in the application--I'd prefer if object references to a Submission or array-type of Submissions always used the variable name "submission" or "submissions," unless there is some specific contextual reason not to.)
  2. I think instead of omitting an icon in the case a message has not been received, an open envelope should be included. The empty space feels inconsistent, and the columns shift when the page is refreshed after reading all unread messages, which I think looks poor.
  3. Similarly, there is now this absent space in rows for outgoing messages. That got me thinking: why not also mark sent messages as read if the source has logged in since they were sent? Since we already expose the rough last login time of each source to the journalists, we might as well inform them if the source has read (or at least had the opportunity to read) their replies. Font Awesome includes envelope-o and envelope-open-o icons, which would help reinforce the visual distinction between replies and submissions.

@redshiftzero redshiftzero force-pushed the mdrose-mark-unread-submissions branch from 700dbe7 to 9964c57 Compare November 15, 2016 08:19
@redshiftzero
Copy link
Contributor Author

  1. Good point! Fixed this.
  2. Note this icon you recommended looks like it was just added in Font Awesome 4.7, so I’ve updated Font Awesome and this now uses this icon to denote the submissions that have been read.
  3. We’re not actually storing the timestamp of the reply as far as I can tell anywhere in the database - we do have the last_access column in the journalists table and last_updated in the sources table, but as soon as a journalist logs in then we are updating last_access. Given that sources are instructed to delete their replies when they read them, and that this is not as straightforward as showing an icon for doc.downloaded (unless I’m overlooking something), I think we should table this until it’s requested. However, to address the aesthetic concern I’ve aligned the replies so that they’re flush with the messages.

Also rebased this on develop. Let me know your thoughts @fowlslegs!

@redshiftzero
Copy link
Contributor Author

Screenshot of how this looks now:

screen shot 2016-11-15 at 12 13 49 am

@psivesely
Copy link
Contributor

  1. So Source.last_updated only gets bumped their key pair is generated via source.async_genkey, or when they make a submission. This is a reasonable choice to help protect source metadata. So if you see that a source has made a new submission, this kind of obviates the need for some other visual indication that they (may) have read your message. So I guess I'm now in favor of how you've got it.

Copy link
Contributor

@psivesely psivesely left a comment

Choose a reason for hiding this comment

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

Nice work!

@psivesely psivesely merged commit 87faa17 into develop Nov 15, 2016
@psivesely psivesely deleted the mdrose-mark-unread-submissions branch November 15, 2016 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants