Make "Highlight As..." sticky across reloads in Diffusion and Differential
ClosedPublic
Actions

Authored by epriestley on Apr 17 2020, 8:16 PM.

Details

Reviewers: None
Maniphest Tasks: T13455: Make "View Options" in Differential sticky across reloads
Commits: rP8aac55cc57cb: Make "Highlight As..." sticky across reloads in Diffusion and Differential

Summary

Ref T13455. Add container-level storage for persistent view state, and persist "Highlight As..." inside it.

The storage generates a "PhabricatorChangesetViewState" configuration object as an output.

When preferences are expressed on a diff and that diff is later attached to a revision, we attempt to copy the preferences.

The internal storage tracks per-changeset settings, but currently always uses "last update wins" to apply the settings in the UI.

Test Plan

Viewed revisions, changed highlighting, reloaded. Saw highlighting stick in revision view and standalone view.
Viewed commits, changed highlighting, reloaded. Saw highlighting stick.
Created a diff, changed highlighting, turned it into a revision, saw highlighting persist.

Diff Detail

Repository

rP Phabricator

Branch

sticky1

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 24152
Build 33259: Run Core Tests
Build 33258: arc lint + arc unit

Event Timeline

epriestley created this revision.Apr 17 2020, 8:16 PM

Owners added a subscriber: Restricted Owners Package.Apr 17 2020, 8:16 PM

Harbormaster completed remote builds in B24152: Diff 50334.Apr 17 2020, 8:17 PM

epriestley requested review of this revision.Apr 17 2020, 8:17 PM

Since these may be prone to heavy usage with arguably minimal value for long-term persistence, you could consider a follow-up revision implementing DifferentialViewStateGarbageCollector.

I think this table won't grow too crazily fast, but it's easy and reasonable to GC so maybe I'll add a GC and just set the default policy to something fairly permissive like "365 days".

This revision was not accepted when it landed; it landed in state Needs Review.Apr 19 2020, 3:58 PM

Closed by commit rP8aac55cc57cb: Make "Highlight As..." sticky across reloads in Diffusion and Differential (authored by epriestley). · Explain Why

This revision was automatically updated to reflect the committed changes.

epriestley added a commit: rP8aac55cc57cb: Make "Highlight As..." sticky across reloads in Diffusion and Differential.