In Progress
Status Update
Comments
ge...@gmail.com <ge...@gmail.com>
ap...@google.com <ap...@google.com> #2
Project: gerrit
Branch: stable-3.7
commit e02812b521d09a314af95f2804c69d2d1cef67df
Author: Jacek Centkowski <geminica.programs@gmail.com>
Date: Thu Feb 29 15:28:06 2024
Fix starred changes clash after repo import from another site
Introduce `virtualId` to change data and initiate it with the
`legacy_id_str` from the index so that changes list screen (which is
populated from the index only) could be properly updated with star bit.
Ensure that `changeServerId` is loaded from notes when notes are
available in the `ChangeData`.
Finally use the `virtualId` in all calls to get/set starred changes so
that the refs clash is avoided. As a result the starred changes ref
gets modified to the following form:
refs/starred-changes/NN/<virtual-id>/<account-id>
where `virtual-id` is `server-id` encoded `change-id` for imported
changes and `change-id` for existing/newly created ones.
Issue 331016670 : to be addressed as the follow-up - toggle star from
changes list screen doesn't work for imported changes - the problem is
that UI useschange.id to obtain the project first which clashes with
existing changes. It works fine on the single change screen.
Bug: Issue 325309573
Release-Notes: Starred changes clash when importing changes from another instance
Forward-Compatible: checked
Change-Id: I85c946efcab5d20dd25682cb758bb9601c93b1d2
M java/com/google/gerrit/extensions/common/ChangeInfo.java
M java/com/google/gerrit/lucene/LuceneChangeIndex.java
M java/com/google/gerrit/server/StarredChangesUtil.java
M java/com/google/gerrit/server/account/AccountResource.java
M java/com/google/gerrit/server/change/ChangeJson.java
M java/com/google/gerrit/server/change/ChangeResource.java
M java/com/google/gerrit/server/change/DeleteChangeOp.java
M java/com/google/gerrit/server/index/change/ChangeField.java
M java/com/google/gerrit/server/query/change/ChangeData.java
M java/com/google/gerrit/server/restapi/account/StarredChanges.java
M javatests/com/google/gerrit/server/query/change/ChangeDataTest.java
https://gerrit-review.googlesource.com/411417
Branch: stable-3.7
commit e02812b521d09a314af95f2804c69d2d1cef67df
Author: Jacek Centkowski <geminica.programs@gmail.com>
Date: Thu Feb 29 15:28:06 2024
Fix starred changes clash after repo import from another site
Introduce `virtualId` to change data and initiate it with the
`legacy_id_str` from the index so that changes list screen (which is
populated from the index only) could be properly updated with star bit.
Ensure that `changeServerId` is loaded from notes when notes are
available in the `ChangeData`.
Finally use the `virtualId` in all calls to get/set starred changes so
that the refs clash is avoided. As a result the starred changes ref
gets modified to the following form:
refs/starred-changes/NN/<virtual-id>/<account-id>
where `virtual-id` is `server-id` encoded `change-id` for imported
changes and `change-id` for existing/newly created ones.
changes list screen doesn't work for imported changes - the problem is
that UI uses
existing changes. It works fine on the single change screen.
Bug:
Release-Notes: Starred changes clash when importing changes from another instance
Forward-Compatible: checked
Change-Id: I85c946efcab5d20dd25682cb758bb9601c93b1d2
M java/com/google/gerrit/extensions/common/ChangeInfo.java
M java/com/google/gerrit/lucene/LuceneChangeIndex.java
M java/com/google/gerrit/server/StarredChangesUtil.java
M java/com/google/gerrit/server/account/AccountResource.java
M java/com/google/gerrit/server/change/ChangeJson.java
M java/com/google/gerrit/server/change/ChangeResource.java
M java/com/google/gerrit/server/change/DeleteChangeOp.java
M java/com/google/gerrit/server/index/change/ChangeField.java
M java/com/google/gerrit/server/query/change/ChangeData.java
M java/com/google/gerrit/server/restapi/account/StarredChanges.java
M javatests/com/google/gerrit/server/query/change/ChangeDataTest.java
ap...@google.com <ap...@google.com> #3
Project: gerrit
Branch: stable-3.7
commit 52aa778774472aa4c91d3b1406f09119073c1ace
Author: Jacek Centkowski <geminica.programs@gmail.com>
Date: Tue Apr 16 08:22:47 2024
Document introduction of `virtual_id_number` to `ChangeInfo`
Note that the underscore (`_`) has been removed from the variable name
to avoid resemblance to the prefix `_` used in `_number`, which
(according to the documentation) denotes deprecation.
Follow-Up-To: I85c946ef
Bug: Issue 325309573
Release-Notes: Add `virtual_id_number` to `ChangeInfo`
Forward-Compatible: checked
Change-Id: Iddf5dee45f80f863ec8dbc113e15e9499bfee757
M Documentation/rest-api-changes.txt
M java/com/google/gerrit/extensions/common/ChangeInfo.java
M java/com/google/gerrit/server/change/ChangeJson.java
https://gerrit-review.googlesource.com/420497
Branch: stable-3.7
commit 52aa778774472aa4c91d3b1406f09119073c1ace
Author: Jacek Centkowski <geminica.programs@gmail.com>
Date: Tue Apr 16 08:22:47 2024
Document introduction of `virtual_id_number` to `ChangeInfo`
Note that the underscore (`_`) has been removed from the variable name
to avoid resemblance to the prefix `_` used in `_number`, which
(according to the documentation) denotes deprecation.
Follow-Up-To: I85c946ef
Bug:
Release-Notes: Add `virtual_id_number` to `ChangeInfo`
Forward-Compatible: checked
Change-Id: Iddf5dee45f80f863ec8dbc113e15e9499bfee757
M Documentation/rest-api-changes.txt
M java/com/google/gerrit/extensions/common/ChangeInfo.java
M java/com/google/gerrit/server/change/ChangeJson.java
Description
Step to reproduce
Expected result
Only C2 of project P2 should be starred
Observed result
Both C1 and C2 are starred.
Background
Starred changes information are stored in
All-Users
in refs like:refs/starred-changes/<changeNumberShard>/<changeNumber>/<accountId>
Since the change number is not unique, all the changes with the same change number will be marked as starred.
Possible solutions are:
virtualId
, akalegacy_id
) instead of the change number in the special refConsidering this fix will need to be done in a stable branch, the first solution is possibly the least impactful.
All the solutions will require a migration strategy.