Fix(watched-projects): Surface existing project errors
Adding a new watched project could fail because, during the add
operation, an access check is performed on the combined list of watched
projects (including the newly added one and existing ones), and this
check can fail if the existing list contains inaccessible projects. This
can happen when a user adds a project to their watched list, and that
project is later made hidden or deleted.
This change surfaces these errors when fetching the watched projects
list. By displaying these errors in the UI, users can identify and
remove the inaccessible projects, resolving the issue and allowing them
to successfully add new watched projects.
Change-Id: I6e55fd3fabc6de315a0e9fbfb280e2e5839c5688
Release-Notes: skip
Google-Bug-Id: b/389630826
Signed-off-by: Milutin Kristofic <milutin@google.com>
diff --git a/Documentation/rest-api-accounts.txt b/Documentation/rest-api-accounts.txt
index cb3517d..1c77e4a 100644
--- a/Documentation/rest-api-accounts.txt
+++ b/Documentation/rest-api-accounts.txt
@@ -3009,6 +3009,7 @@
|Field Name | |Description
|`project` | |The name of the project.
|`filter` |optional|A filter string to be applied to the project.
+|`problem` |optional|An error message when project is for example hidden or deleted.
|`notify_new_changes` |optional|Notify on new changes.
|`notify_new_patch_sets` |optional|Notify on new patch sets.
|`notify_all_comments` |optional|Notify on comments.
diff --git a/java/com/google/gerrit/extensions/client/ProjectWatchInfo.java b/java/com/google/gerrit/extensions/client/ProjectWatchInfo.java
index 8f5af76..c446d87 100644
--- a/java/com/google/gerrit/extensions/client/ProjectWatchInfo.java
+++ b/java/com/google/gerrit/extensions/client/ProjectWatchInfo.java
@@ -19,6 +19,7 @@
public class ProjectWatchInfo {
public String project;
public String filter;
+ public String problem;
public Boolean notifyNewChanges;
public Boolean notifyNewPatchSets;
@@ -32,6 +33,7 @@
ProjectWatchInfo w = (ProjectWatchInfo) obj;
return Objects.equals(project, w.project)
&& Objects.equals(filter, w.filter)
+ && Objects.equals(problem, w.problem)
&& Objects.equals(notifyNewChanges, w.notifyNewChanges)
&& Objects.equals(notifyNewPatchSets, w.notifyNewPatchSets)
&& Objects.equals(notifyAllComments, w.notifyAllComments)
@@ -46,6 +48,7 @@
return Objects.hash(
project,
filter,
+ problem,
notifyNewChanges,
notifyNewPatchSets,
notifyAllComments,
@@ -61,6 +64,9 @@
if (filter != null) {
b.append("%filter=").append(filter);
}
+ if (problem != null) {
+ b.append("%problem=").append(problem);
+ }
b.append("(notifyAbandonedChanges=")
.append(toBoolean(notifyAbandonedChanges))
.append(", notifyAllComments=")
diff --git a/java/com/google/gerrit/server/restapi/account/GetWatchedProjects.java b/java/com/google/gerrit/server/restapi/account/GetWatchedProjects.java
index 00c70d0..8f047a3 100644
--- a/java/com/google/gerrit/server/restapi/account/GetWatchedProjects.java
+++ b/java/com/google/gerrit/server/restapi/account/GetWatchedProjects.java
@@ -26,6 +26,7 @@
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
+import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountResource;
@@ -35,6 +36,7 @@
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.restapi.project.ProjectsCollection;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -53,13 +55,18 @@
private final PermissionBackend permissionBackend;
private final Provider<IdentifiedUser> self;
private final Accounts accounts;
+ private final ProjectsCollection projectsCollection;
@Inject
public GetWatchedProjects(
- PermissionBackend permissionBackend, Provider<IdentifiedUser> self, Accounts accounts) {
+ PermissionBackend permissionBackend,
+ Provider<IdentifiedUser> self,
+ Accounts accounts,
+ ProjectsCollection projectsCollection) {
this.permissionBackend = permissionBackend;
this.self = self;
this.accounts = accounts;
+ this.projectsCollection = projectsCollection;
}
@Override
@@ -84,11 +91,16 @@
.collect(toList()));
}
- private static ProjectWatchInfo toProjectWatchInfo(
+ private ProjectWatchInfo toProjectWatchInfo(
ProjectWatchKey key, ImmutableSet<NotifyType> watchTypes) {
ProjectWatchInfo pwi = new ProjectWatchInfo();
pwi.filter = key.filter();
pwi.project = key.project().get();
+ try {
+ var unused = projectsCollection.parse(key.project().get());
+ } catch (RestApiException | IOException | PermissionBackendException e) {
+ pwi.problem = e.getMessage();
+ }
pwi.notifyAbandonedChanges = toBoolean(watchTypes.contains(NotifyType.ABANDONED_CHANGES));
pwi.notifyNewChanges = toBoolean(watchTypes.contains(NotifyType.NEW_CHANGES));
pwi.notifyNewPatchSets = toBoolean(watchTypes.contains(NotifyType.NEW_PATCHSETS));
