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

Search: Allow marking notes as restricted #2304

Merged
merged 8 commits into from Dec 10, 2018

Conversation

Projects
None yet
2 participants
@kevinrobinson
Copy link
Contributor

kevinrobinson commented Dec 10, 2018

Who is this PR for?

students, families. first users SHS counselors

What problem does this PR fix?

If notes with sensitive information are entered but not marked as restricted, there's no way for educators to resolve this without emailing for help. This works well and they can do this quickly, but we should support educators doing this directly.

Adding search magnifies this problem since it's simpler to search for particular topics that might be sensitive, and so we can use search to enable educators to resolve any privacy concerns for students before releasing it more widely.

What does this PR do?

Adds a "Mark restricted" link to notes on the search page only.

This involves changing FeedView to support threading per-note props at render time, and then adds a new MutableFeedView that adds in this UI element and handles making server requests and optimistically updating the local UI as well. This PR then makes use of this in SearchNotesPage only. The metadata for search results doesn't update in response to this, and that's something to improve in a separate PR; this is aimed at enabling the most critical use case only.

This also fixes an existing bug in SelectTimeRange impacting the search page - the momentRange fn was mutating a moment that it was passed, which led to excluding notes from the current day in search.

Screenshot (if adding a client-side feature)

screen shot 2018-12-10 at 8 47 59 am

screen shot 2018-12-10 at 8 48 04 am

Checklists

Which features or pages does this PR touch?

  • Home page
  • Search notes

Does this PR use tests to help verify we can deploy these changes quickly and confidently?

  • Included specs for changes
  • Manual testing made more sense here
@studentinsights-bot

This comment has been minimized.

Copy link

studentinsights-bot commented Dec 10, 2018

@kevinrobinson, this looks like it might be worth double-checking! @kevinrobinson might be able to help.

@kevinrobinson

This comment has been minimized.

Copy link
Contributor Author

kevinrobinson commented Dec 10, 2018

selfie

@kevinrobinson kevinrobinson merged commit f0c2411 into master Dec 10, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@kevinrobinson kevinrobinson deleted the feature/mark-notes-as-restricted branch Dec 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment