[Xzzz]

Summary

Stops writing flag changes to bugs_activity: new ones are only recorded in flag_activity, and everything that reads flag changes now reads both tables, bugs_activity for the older changes and flag_activity for the newer ones.

Part 2 of 1806896, whose part 1 (#2657) moved flag_state_activity to core as flag_activity and writes every flag change to both tables.

Changes

• Bugzilla::Bug and Bugzilla::Attachment no longer log flagtypes.name to bugs_activity; the user merge in editusers.cgi writes its flag changes to flag_activity.
• New flag_activity.who column: the user who made the change. It differs from setter_id when a request is redirected, which keeps the original setter. Existing rows get who = setter_id. Indexed with flag_when.
• checksetup gives each flag that has no flag_activity row (flags last changed before flag_activity logged every type) a row with its current state, dated at its last change, in batches of 10,000 flags. History and bugmail don't show it, but the flag's next change gets its previous state.
• Bug history (GetBugActivity), bugmail (_get_diffs) and the BMO user activity report read the flagtypes.name rows in bugs_activity, plus the flag_activity rows newer than the bug's last one (Bugzilla::FlagActivity->newer_than_bugs_activity_sql), so changes written to both tables are listed once. A request that changes a flag several times shows its net change, compared with the flag's last row before the request.
• Search changedbefore/changedafter/changedfrom/changedto/changedby/everchanged on flags, and the ReleaseTracking report, match either table.
• The BugModal activity stream finds a flag's last change by its flag_id in flag_activity.
• The other readers of a user's activity also read flag_activity: the non-bot last change time, the UserProfile stats, the TagNewUsers first activity, owner idle time, and changed* searches on any field.
• A flag type that has flag_activity rows can only be deactivated, not deleted, as its history would be deleted with it. The user deletion check in editusers.cgi counts flag_activity rows, and scripts/remove-non-public-data.pl keeps who.
• New tests: t/flag-activity-history.t, -search.t, -who.t, -user-stats.t, -stream.t, -start-rows.t and t/flag-type-delete.t.

The old flagtypes.name rows stay in bugs_activity, nothing is deleted.

Test plan

• Set a needinfo, a review on an attachment and an approval flag, then clear one: bug history, the activity stream and the bugmail show each change with its previous state, and bugs_activity gets no new flagtypes.name row
• Redirect a needinfo to someone else as another user: history shows that user
• On a bug with flag changes from before the deploy: history shows them once, with their previous state
• Search "Flags changed to needinfo?" and the user activity report find both older and new changes
• Merge two users with pending requests in editusers.cgi: the reassigned requests appear in bug history
• Delete a flag type that was used: only deactivation is offered

References

• Bugzilla link: 2085398
• Part 2 of 1806896

[Xzzz]

Please note that the test_sanity critic failure is the $$ on master fixed by #2795. I'll re-run CI once it's merged, it should go green, unless I missed stg ;)

[Xzzz]

Also in this push, fixed some issues and optimized SQL request:

• c560c65: until July, flag_activity only logged review, feedback and needinfo, so other flags last changed before then had no row, and their next change showed no previous value (a clear disappeared from history). checksetup now gives each flag that has no row a starting one, with its current state, dated at its last change, in batches of 10,000 flags. History and bugmail don't show it, since it isn't newer than that change in bugs_activity.
• aca6a3b: bugmail skips flag changes that change nothing (a flag created and removed in one request), as history already did.
• f6a636a: changedfrom / changedto on any field also match flag changes.
• 98f9ae8: owner idle time uses three EXISTS instead of three joins, which multiplied each other's rows for active assignees.

The PR description is updated too.

[dklawren]

flags.modification_date is nullable but flag_activity.flag_when is NOT NULL, so a single flag with a null date makes checksetup die here, maybe use COALESCE(modification_date, creation_date)

[dklawren]

this walks every flag with a NOT EXISTS probe on every checksetup run, even after the backfill is done, could it be guarded so it only runs once

[dklawren]

this correlated subquery is re-run for each flag_activity row and scans all of the bug's bugs_activity rows (there is no (bug_id, fieldid) index), which adds up in the user activity report and the product stats, maybe compute the cutoff once per bug or add an index

[dklawren]

flag_activity has no index on setter_id, so this OR does a full table scan on the delete confirmation page

[dklawren]

a request that creates and clears a flag at once shows up here as an empty change, since unlike history and bugmail this report doesn't skip rows where removed equals added