Bug 1806896 - Move flag_state_activity to core as flag_activity#2657
Bug 1806896 - Move flag_state_activity to core as flag_activity#2657Xzzz wants to merge 11 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Moves flag change-tracking from the Review extension into Bugzilla core by introducing a new core ORM (Bugzilla::FlagActivity) and core schema table (flag_activity), while updating extension and BMO scripts to use the new class/table name.
Changes:
- Add core
Bugzilla::FlagActivityORM and coreflag_activityDB schema table. - Update
Bugzilla::Flagto write flag create/update/delete activity rows toflag_activity. - Update Review/BMO tooling and provide a compatibility shim for the old extension class name.
Reviewed changes
Copilot reviewed 7 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/remove-non-public-data.pl | Switches sanitization logic from flag_state_activity to flag_activity. |
| extensions/Review/lib/WebService.pm | Updates REST endpoint implementation to use Bugzilla::FlagActivity and renamed helper. |
| extensions/Review/lib/FlagStateActivity.pm | Converts old extension class into a thin compatibility shim inheriting from core. |
| extensions/Review/Extension.pm | Removes redundant activity logging hooks and adds migration logic for the renamed table. |
| extensions/BMO/bin/export_bmo_etl.pl | Updates ETL export path to pull deleted flags from flag_activity. |
| extensions/BMO/bin/bug_1022707.pl | Updates maintenance script to update flag_activity instead of flag_state_activity. |
| Bugzilla/FlagActivity.pm | Adds new core ORM for the flag_activity table. |
| Bugzilla/Flag.pm | Writes all flag lifecycle events into flag_activity. |
| Bugzilla/DB/Schema.pm | Adds new core flag_activity table definition and indexes. |
Comments suppressed due to low confidence (1)
extensions/Review/lib/WebService.pm:273
- This helper was renamed to _flag_activity_to_hash, but the parameter is still named $fsa (from the old FlagStateActivity naming). Renaming it avoids confusion and makes the code match the new class/table terminology.
sub _flag_activity_to_hash {
my ($self, $fsa, $params) = @_;
my %flag = (
id => $self->type('int', $fsa->id),
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| use Bugzilla::Error qw(ThrowUserError); | ||
| use Bugzilla::Util qw(trim datetime_from); | ||
| use List::MoreUtils qw(none); |
There was a problem hiding this comment.
Fixed by modifying line 14 to use Bugzilla::Error qw(ThrowCodeError ThrowUserError);
There was a problem hiding this comment.
Or just use use Bugzilla::Error; which will auto import them. This is what is done in most places in the code already.
There was a problem hiding this comment.
Line modified to use Bugzilla::Error; (commit "Bug 1806896 - Fix missing ThrowCodeError import in FlagActivity.pm")
| # Bug 1806896 - rename flag_state_activity to flag_activity (now in core schema) | ||
| if ($dbh->bz_table_info('flag_state_activity') | ||
| && !$dbh->bz_table_info('flag_activity')) | ||
| { | ||
| $dbh->do('RENAME TABLE flag_state_activity TO flag_activity'); | ||
| } |
There was a problem hiding this comment.
Fixed in "Bug 1806896 - Fix flag_state_activity migration and use FK-handling helpers" commit
| INDEXES => [ | ||
| flag_activity_flag_id_idx => ['flag_id'], | ||
| flag_activity_type_id_idx => ['type_id'], | ||
| flag_activity_bug_id_idx => ['bug_id'], | ||
| ], |
There was a problem hiding this comment.
Fixed in "Bug 1806896 - Add status/flag_when index to flag_activity" commit
Summary
Moves flag change tracking infrastructure from the Review extension into Bugzilla core, and renames the table from
flag_state_activitytoflag_activity.Bugzilla::FlagActivityas a core ORM class (moved fromBugzilla::Extension::Review::FlagStateActivity)flag_activitytable toBugzilla::DB::Schema;checksetup.plrenames the existingflag_state_activitytable on first runBugzilla::Flagnow writes all flag changes toflag_activity(create, update, delete) - previously onlyreview,feedback,needinfoanddata-reviewwere tracked_log_flag_state_activity), which are now redundant; request-count logic (_is_countable_flag) is unchangedWebService.pm,export_bmo_etl.pl,bug_1022707.plandremove-non-public-data.plto reference the new class and table nameBugzilla::Extension::Review::FlagStateActivityis kept as a thin backward-compat shim inheriting fromBugzilla::FlagActivitybugs_activitycontinues to recordflagtypes.nameentries during the dual-write period.A follow-up PR will stop the
bugs_activityflag writes and switch all consumers (BugMail,GetBugActivity,ActivityStream) to read exclusively fromflag_activityonce changes has been validated in production.Test plan
checksetup.pland verifyflag_state_activityis renamed toflag_activityflag_activityfor all of them (not just review/feedback/needinfo/data-review)bugs_activitystill recordsflagtypes.nameentries as beforeGET /rest/review/flag_activitystill returns correct results