studentinsights icon indicating copy to clipboard operation
studentinsights copied to clipboard

Profile v3: Show presence of restricted notes inline, require action to view or edit

Open kevinrobinson opened this issue 6 years ago • 0 comments

Part of https://github.com/studentinsights/studentinsights/issues/1990.

Scope

Notes can influence a few different places:

  • feed
  • student profile
  • full case history
  • my notes
  • PDF export
  • absences or tardies dashboard
  • insights boxes (grades, absences)
  • school roster
  • homeroom page
  • section page

Approach

For restricted notes, right now they only appear in a separate page. We'd like to move them inline so that the presence of a restricted note (but not the substance) is more visible, and to make less friction for folks writing a note to tag is as "restricted."

Here's some work to migrate to inline restricted notes. Since there are so many exposure points, the overall strategy is to:

  • [x] 1. add tighter control over accessing restricted note content on the server (primarily in EventNote#as_json) (https://github.com/studentinsights/studentinsights/pull/1999)
  • [x] 2. more explicitly allow access to restricted note content in the existing product only in places that need it (ie, only restricted notes page) (https://github.com/studentinsights/studentinsights/pull/1999)
  • [x] 3. remove any unnecessary exposure in existing product (eg, serialized to people who have access but don't need that data, remove serialization event_note_revisions from endpoints) (https://github.com/studentinsights/studentinsights/pull/1999)
  • [x] 4. ship profile v3 without view/edit (https://github.com/studentinsights/studentinsights/pull/1999)
  • [x] 5. ship view in profile v3 (https://github.com/studentinsights/studentinsights/pull/2004)
  • [x] 6. incorporate or migrate restricted transition notes (https://github.com/studentinsights/studentinsights/pull/2008)
  • [x] 7. remove restricted notes page
  • [ ] 8. incrementally update other parts of the product

Particular places within the product

a. Profile v3, notes

Aiming to make the minimal changes here:

  • [x] show presence of restricted notes (https://github.com/studentinsights/studentinsights/pull/1999)
  • [x] with access, clicking allows viewing content of restricted note (https://github.com/studentinsights/studentinsights/pull/2004)
  • [x] on the write path, can add a restricted note (https://github.com/studentinsights/studentinsights/pull/2004)
  • [ ] with access, clicking allows editing existing restricted note

b. Profile v2

  • [x] ensure no leakage regressions (https://github.com/studentinsights/studentinsights/pull/1999)
  • [x] remove restricted notes page and endpoints

c. My notes

To start, this will continue to only show restricted notes that the current user has written and they'll be readonly.

  • [x] show presence of restricted notes (https://github.com/studentinsights/studentinsights/pull/1999)
  • [x] show readonly content of restricted notes you wrote (https://github.com/studentinsights/studentinsights/pull/1999)
  • [x] refactor to use the same mechanism as the profile (redacted, click to see content and possibly edit).

d. Homeroom page

This doesn't include restricted notes (event_notes_without_restricted) so this is safe but will appear mismatched to folks clicking into the profile.

  • [ ] update to include presence of restricted notes

e. Section page

This doesn't include restricted notes (event_notes_without_restricted) so this is safe but will appear mismatched to folks clicking into the profile

  • [ ] update to include presence of restricted notes

f. Home page, feed

The Home page feed does not show restricted notes. We could update this to show the presence (or allow viewing/editing inline with access).

  • [ ] show presence in feed. This might involve some UI unification, since the display of notes in the Feed and the Profile use different components.

g. Home page, insights boxes

InsightStudentsWithHighAbsences and InsightStudentsWithLowGrades both exclude restricted notes in their query, but with this change, they should include them (and continue not sending any content).

  • [ ] update InsightStudentsWithHighAbsences
  • [ ] update InsightStudentsWithLowGrades

h. PDF export

The PDF export never includes restricted notes, even if the user has access to see them. We could update but not a priority.

  • [ ] update PDF export to show presence of restricted note (but not content)

i. School roster

The school roster includes restricted notes, but strips out the content even if the user has access to see it. This is what we'd like to do in the Homeroom and Section page. Nothing to do here.

j. Absences, Tardies, Discipline dashboards

These already include restricted notes, but don't send any content down. Nothing to do here.

k. Profile v3, full case history

This is overhead, since it's a different set of rendering code than in the feed or profile.

  • [x] show presence with server <redacted> text (https://github.com/studentinsights/studentinsights/pull/1999)
  • [ ] update to show message and work similarly to profile notes or feed

l. Tiering

It doesn't include restricted notes. We should update this if we move forward with this, but not a priority.

  • [ ] update to include presence of restricted notes, but no content

Other details or questions

Does EventNoteRevision leak text as well? Yep. We can fix this by removing it from the controller/UI altogether. And add another layer by guarding #as_json on the model. Ideally we'd remove the parallel tables but not now.

  • [x] remove from controller/UI (https://github.com/studentinsights/studentinsights/pull/1999)
  • [x] add model-layer guard (https://github.com/studentinsights/studentinsights/pull/1999)
  • [ ] migrate to simplify from parallel tables?

What about TransitionNotes? Going to punt that to a separate pass.

  • [x] do another pass on TransitionNotes (https://github.com/studentinsights/studentinsights/pull/2008)

What about attachments? Yep, this would leak. But there are no usages of this, and there's another layer of authorization for URL itself (eg, in Drive), so going to punt this to later (alternately could remove attachments altogether since it's not doing more than just a plain URL in the text).

  • [ ] tighten up restricted attachments (or remove feature)
  • [ ] add attachments to <RestrictedNotesPresence /> when viewing (or remove feature)

What about existing endpoints for interacting with event notes? Yep, these don't enforce authorization rules and could conceivably allow someone without access to create a restricted note (but not gain access or change the status of a note).

  • [x] tighten up event_notes_controller endpoints (https://github.com/studentinsights/studentinsights/pull/1999)

kevinrobinson avatar Aug 21 '18 22:08 kevinrobinson