freedomofpress / securedrop-client

a Qt-based GUI for SecureDrop journalists 📰🗞️
GNU Affero General Public License v3.0
40 stars 39 forks source link

Highlight sources with unseen source submissions #187

Closed eloquence closed 3 years ago

eloquence commented 5 years ago

User Story

As a journalist, I want to know at a glance which sources have unseen submissions, so that I can quickly identify new information that may require attention.

Justification

Seen/unseen status is a fundamental paradigm in messaging applications. The feature exists in the pre-workstation user experience (as a global status of messages and documents, internally referenced as is_downloaded, and shown in the Journalist Interface UI as bolded/unbolded). User research has validated the need for similar functionality in the SecureDrop Client.

Mockup

(Design specification to come)

unseen-mockup

Terminology:

Source list: the panel on the left in the client view, which shows a list of sources by codename (e.g., "benign artichoke")

Conversation view: the panel on the right, which shows the messages, documents and replies for the currently selected source

Submissions: Messages or files sent by a SecureDrop source

Unseen submissions: Messages or files that have not been displayed in the conversation view of the SecureDrop Client, or downloaded via the Journalist Interface, by any user

Acceptance Criteria

Basic seen/unseen status and synchronization

Given that there are sources with unseen submissions on the server When the client retrieves these submissions as part of a sync And those sources are not actively selected Then these sources should be displayed as unseen in the source list

Given that there is a source with unseen submissions on the client And that all submissions from this source are marked as seen on the server When the client synchronizes with the server Then the source should be displayed as seen in the source list

Explanation: The information on the server is canonical; the same user may be connecting from more than one workstation.

Selecting a source with unseen submissions

Given that there is a source with unseen submissions on the client When I select that source in the source list Then the source should immediately be displayed as seen in the source list And the client should record all the submissions from that source as seen by the logged in user

Explanation: Recording a change in the status implies a server-side API request followed by a client-side database update.

Error case

Given that there is a source with unseen submissions on the client When I shut down the server while the client is running And I select that source Then the source should immediately be displayed as seen in the source list And the client should keep retrying pending server operations indefinitely And a generic network error should be displayed to the user until network access is restored

Explanation: We optimistically update the UI after the user selects a source, and keep retrying indefinitely if there's a network problem.

Offline usage

Given that there are sources with unseen submissions in the local client database When I use the client in offline mode *Then* all sources should be displayed in a read-only state (TBD) in the list source list And** seen/unseen status should not be modified as a result of my actions

Deleted journalist

Given that there is a source whose submissions have all been seen by a single journalist When I delete that journalist user through the Admin Interface Then the source should still be displayed as seen in the source list on the client after the next sync

Replies

Given that I have selected a source with unseen submissions and changed its status to seen When I send a reply to that source Then the source should continue to be displayed as seen in the source list

Explanation: Replies do not impact the seen/unseen status in the source list

Given that there is a source with no unseen submissions on the client When another journalist sends a reply to that source Then the source should continue to be displayed as seen in the source list after a sync

Explanation: As above, replies do not impact the seen/unseen status in the source list

Journalist Interface

Given that there is a source with unseen submissions on the server When I initiate a download of these submissions via the web-based Journalist Interface Then the source should be displayed as seen in the source list on the client after the next sync

Explanation: Downloading via the Journalist Interface counts as "seeing" a message or file.

Updated per user research review & team discussions - 2020-08-26 - @eloquence

ninavizz commented 5 years ago

I've died and gone to Gherkin heaven!

eloquence commented 5 years ago

Just a note that we've tentatively agreed that we'll aim for tracking this on a per-user basis from the start, instead of using the existing global read/unread flag (this information would be stored separately and independently on the server). Will rewrite the issue accordingly & file a corresponding server-side issue.

Update: As part of this change, I've also changed the language to new/seen/unseen, to set it apart from the language we use in the web UI, and to foreshadow additional "Seen by" functionality.

redshiftzero commented 5 years ago

This issue should address @creviera's note on #240 regarding the following situation:

[Unseen Submission] Depending on how large the journalist's reply is, the source submission that was sent before the reply might not appear in the conversation box when the journalist refreshes. It'll appear in the middle of the conversation somewhere and that could be outside the scope of the conversation box.

eloquence commented 5 years ago

@redshiftzero @creviera @ninavizz

This issue has been reworded significantly over the last couple of days. Allie, as you make progress on your sprint commitments, I suggest we schedule a chat to go over this.

This issue is becoming quite complex; as we get a handle around the server-side changes that are required, we may discover a logical way to split it.

Regarding the reply issue Allie flagged in her review of #240, if I understand correctly, the issue is that replying will potentially scroll previously received messages/documents out of view. If so, I think that's fine and intended behavior, consistent with other messaging applications. I believe it's important, however, that we scroll new activity into view when you select the source to reply to, and that's reflected in the acceptance criteria above.

This "scroll new activity into view" behavior would only occur the first time; if you then un-select and re-select the source, it would scroll to the bottom. That's consistent, e.g., with the behavior in Slack or other applications designed to make new activity easily identifiable.

We've not yet fully user-tested the scrolling behavior and the "seen" indicator for files, so those are good areas to consider for the next round of testing as well.

sssoleileraaa commented 5 years ago

@eloquence - yes, the issue I was referring to can be compared to responding to a Slack message inline by starting a thread - sometimes it can be challenging to know where in the conversation a new message was added so they have an All Threads tab to see any changes that you might have missed.

In the case of SecureDrop, this is less likely to happen since message sync should happen every 5 minutes (and there isn't a threads feature). But it could happen when a journalist sends a bunch of replies or a long reply that pushes a lot of conversation history into backscroll around the same time that a source is submitting message(s) or a document(s).

Scrolling to the first occurrence of new activity the first time a source is selected helps indicate to the journalist that they should read from that point on, but it would also be useful (post beta) to have some sort of way to indicate which new submissions came in from the server that are now outside the scroll view for the scenario where it's not the first time a source is selected.

As you mentioned, the "seen" indicator for files could potentially address this issue. And we might want to consider adding unseen messages to the indicator.

eloquence commented 5 years ago

But it could happen when a journalist sends a bunch of replies or a long reply that pushes a lot of conversation history into backscroll around the same time that a source is submitting message(s) or a document(s).

Intuitively, my sense is that users would commonly notice without an additional notification (most messengers "de-escalate" notification levels for conversations that are in focus) due to the keyboard<->screen eye movement that is common during message composition for people who are not touch typists, and the screen focus that is typical for people who are. (Down the road if we can develop for accessibility, we'd want to make sure that a real-time voice readout occurs for blind users.)

I fully agree it's worth carefully thinking about the different options of no notification vs. ephemeral notification vs. persistent notification for this case.

This also raises the important general point of how the client should behave when a message or document arrives from a source while the source is selected. Assuming we can determine whether the client window is actively selected or not, I believe the behavior should likely be different -- i.e. we need to do more to get the user's attention if the client window is not selected but the source is. Will flesh out a bit along those lines.

(Update: Updated the spec a bit more to account for "currently selected" status. Whether the client window is selected or not adds significant complexity and I'm okay keeping that out of scope for now, assuming that the client is primarily used as a foreground application or not at all. But we should probably revisit that in an iteration.)

ninavizz commented 5 years ago

So: I want to be very, very careful with patterns we pick-up from other apps... and to inject a cautionary segue here, as I think it's very relevant to this discussion.

GMail and Slack are two app suites that target a broad enterprise audience. Unlike Microsoft, those teams are not led by MBA/Sales, but by UX-favoring product teams. So: while Microsoft's UX research efforts are very well funded and also target their broad enterprise base... Microsoft product teams have a ways to go, still, with embracing a "user first" approach in bridging the gap between research and what gets shipped. Microsoft products ship to meet the desire of their direct customers (IT executives, not the actual users) whereas Slack and Google "get" and embrace who actual users (and what their needs) are.

Messaging apps that are limited to the early-adopter crowd, hackers, techies, and security/privacy enthusiasts, are often not influenced by much user research. Signal has had an open "UX Lead" role on their Jobs board for almost 2 years now—so I doubt its product is influenced much from what is actually usable, vs what the developers chose to do. Discourse, I don't know much about... but the fact that it's a FOSS product whose team is mostly white dudes, and is a product I've only ever seen used for techie forums, has me doubting much research informs its shaping.

Slack, on the other hand, was created as very much the "early adopter crowd" app, but is now far more heavily influenced by usability research in what goes to market. Their micro-interactions are all very carefully studied, and I trust what both them and GMail put into their products, when in doubt, purely because of how I understand their research and product team dynamics (and investments in user research) to be. FYI.

sssoleileraaa commented 5 years ago

Just a note that we've tentatively agreed that we'll aim for tracking this on a per-user basis from the start, instead of using the existing global read/unread flag (this information would be stored separately and independently on the server). Will rewrite the issue accordingly & file a corresponding server-side issue.

Following up from a conversation I just had with @eloquence, we might also want to consider adding replies as new activity since they can come from different journalists signed in with different accounts.

sssoleileraaa commented 5 years ago

Also, bumping this again:

[Unseen Submission] Depending on how large the journalist's reply is, the source submission that was sent before the reply might not appear in the conversation box when the journalist refreshes. It'll appear in the middle of the conversation somewhere and that could be outside the scope of the conversation box.

After discussing this more with @eloquence, rather than continue the current behavior where a source submission could be inserted in the middle of a conversation, we should always display the new activity at the bottom of the conversation view, even if that means we need to rename the file so it's ordered correctly. I will create an Issue that describes the desired behavior in more detail.

ninavizz commented 5 years ago

UX Spec: File Read

Checkmark SVG: https://drive.google.com/open?id=1uAFYxeQho-ebFJI0ulT4bpzWJu9yPxri

Per securedrop-ux/#39, the 'lil checkmark to the left of the filename will be the visual indicator. Ideally it will only turn-on, when the user clicks into a disp-VM that the file was opened within... but I suspect that's not possible to track w/in Qubes. If not, then I guess just turn it on 30 seconds after the user triggers the "View" action... assuming it doesn't crash Qubes? Heh.

eloquence commented 5 years ago

The plan of record, as discussed in the client sync meeting on 3/25, is that we may be able to initially build out this functionality as an (ephemeral) client-side feature. This will ensure that we can ship a version of this feature with the pilot, with a full client/server implementation as a stretch goal for the beta.

This would still require per-user storage of this data on the client, but all acceptance criteria related to the server state can be omitted for an initial implementation. I will create a stripped down set of acceptance criteria closer to implementation.

sssoleileraaa commented 5 years ago

Today, we discussed saving state before exiting the application so we know which source conversation to display when the application starts up again. More detail can be found here: https://github.com/freedomofpress/securedrop-client/issues/336

ninavizz commented 4 years ago

Blowing off some dust, and adding to #650

eloquence commented 4 years ago

We decided a while ago that this is Post-Beta (or possibly Mid-Beta), so moved off milestone & near-term backlog for now.

eloquence commented 4 years ago

These two components can be safely implemented independently:

To make this more manageable, I think we should split this issue accordingly, and prioritize them independently.

We will also need to decide if we want to do a client-side-only implementation at first, or immediately go with a client/server implementation. Regardless, we will have to account for multiple users using the same workstation. In other words, different sources will be "new" depending on who is logged in.

sssoleileraaa commented 4 years ago

When we were discussing this feature back and forth over a year ago (just reread to refresh my memory) the client was a lot different so some of the discussions earlier on were centered around old behavior like the 5-minute refresh cycle and the losing position in a conversation due to auto-scrolling. To make it easier to track/follow discussions around each feature (there is a lot of conversation not documented here, plus it's pretty easy to forget), I think we should split this up even further than what is suggested above. What do you think about this split @eloquence:

(the two @eloquence already mentioned)

  • "new" (e.g., bolding in the source list)
  • "seen" at the document level (e.g., checkmark at the document level)

(and...)

We will also need to decide if we want to do a client-side-only implementation at first, or immediately go with a client/server implementation. Regardless, we will have to account for multiple users using the same workstation. In other words, different sources will be "new" depending on who is logged in.

I think client-side-only implementation to start off with makes sense. Once the server-side is ready, we can handle synchronization similar to the way we handle it with starring.

eloquence commented 4 years ago

Yeah, I'm in favor of a granular split into small improvements that can be delivered independently. Before we reorganize this issue, I'd like to get some input from our users, especially on the question if read/unread status should be global or per-user (as validated by having them play with prototypes that show different behaviors).

That could also influence the implementation strategy -- we do have a basic read/unread state on the server now (called downloaded on the server and exposed as is_read via the API, but only available for messages/submissions), so if we go with a "shared inbox / global state" model, there may be a relatively straightforward client/server implementation path.

sssoleileraaa commented 4 years ago

I agree we need more info. I think it would also be helpful to show a demo of what could happen when there are say two or three users on the same team working remotely at the same time.

ninavizz commented 4 years ago

@creviera All of the above need to be solved-for in a prototype for adequately testing all of this; as our users will have the same exact concerns. Which is why I'm cramping my brain trying to learn some React atm, so I can make more high-fidelity prototypes. Have also been thinking about how this could also make for easier ux/dev handoffs, and possibly even reduce... anyway, still trying to see if I can do this. :)

sssoleileraaa commented 4 years ago

@ninavizz I'd also be interested in going to these prototype sessions with the users to observe if you're cool with that, and I can help contribute code for prototyping if that helps. The global inbox scenario where there are multiple users using the shared inbox at the same time sounds pretty difficult to prototype, so if we want to switch to demo builds of the client, let me know.

eloquence commented 4 years ago

Here's where we currently are with this issue:

1) A first round of user research with pilot users has been completed. In this research, we're focusing on better understanding users' mental model of the SecureDrop inbox, whether read/unread state should be global or per-user, and how read/unread will interact with "newness" of sources.

2) We're reaching out to additional research participants from other organizations, and have updated to the prototype we use for user-testing to incorporate a "newness" indicator alongside read/unread status.

3) During the 8/6-8/20 sprint, we'll aim to complete additional user research (pending research participant availability) and to map out one or more implementation scenarios in more detail, to better understand server-side requirements which we may need to deliver as part of the SecureDrop 1.6.0 release (scheduled for 9/22).

Please consider the top-level description of this issue a placeholder until we've made a final decision on the scope of the first iteration, at which point we can update it, or split off sub-issues, as appropriate.

eloquence commented 4 years ago

Quick update:

eloquence commented 4 years ago

Further scoping discussion and review of user research to-date will happen early in in 8/20-9/2 sprint.

We did this today, see meeting notes here (cross-linked from Client Wiki as well). We'll do a small work session tomorrow to update the acceptance criteria in this issue (and likely split off some chunks of it), and then we'll also have to add up-to-date design specifications.

sssoleileraaa commented 4 years ago

In addition, here is the Seen/Unseen V1 Spec which is a working document. You'll see a v2 doc linked to there to capture ideas that are not going to make it into v1, as we discussed today. We'll see what tomorrow's threat modelling session tomorrow yields. more to come!

eloquence commented 4 years ago

We've revised the acceptance criteria in this issue consistent with recent discussions & emerging team consensus. We've not decided the behavior in offline mode yet -- we may want to preserve display of the seen/unseen status without altering it, perhaps using a different style for source display & selection. @ninavizz will do some design explorations around that.

eloquence commented 4 years ago

Just a note that after review with @ninavizz I made one small addition to the acceptance criteria, for replies by another journalist that come in via a sync:

Given that there is a source with no unseen submissions on the client When another journalist sends a reply to that source Then the source should continue to be displayed as seen in the source list after a sync

This is just to make it clearer that journalist replies, whether by the logged in user or another user, will never cause a source to be displayed as unseen.

eloquence commented 3 years ago

Our expectation is that this feature will land after the major template consolidation release (which will include other unreleased changes in the SecureDrop Client, including reply badges). During the 10/15-10/28 sprint, @creviera will do timeboxed implementation work on a draft PR that implements the basic functionality outlined in this issue.

ninavizz commented 3 years ago

https://zpl.io/a8LL5eX

@creviera This Zeplin has the correct values for seen/unseen wrt font weights and coloration. The most notable things I can think of, are as follows:

w00p w00p!

eloquence commented 3 years ago

(Removed from project board as #1165, which is tracked, is expected to fully resolve.)

ninavizz commented 3 years ago

@creviera Did the above design particulars make it into the linked PR?

sssoleileraaa commented 3 years ago

https://github.com/freedomofpress/securedrop-client/issues/187#issuecomment-711098294 lgtm! we can review the client together during a demo screenshare so you can see how it looks live.

ninavizz commented 3 years ago

Before doing a demo screenshare I'd first like to make sure the above specs are implemented into the code, so I can know what we're looking at. No rush! Just don't want it left behind with this Issue closed... so consider this more a flag for @eloquence. ;)