feat: project-wide bookmark manager with a -/bookmarks tab and home page section

admin/database/database.go

@@ -300,6 +300,8 @@ type DB interface {
 	InsertProjectAccessRequest(ctx context.Context, opts *InsertProjectAccessRequestOptions) (*ProjectAccessRequest, error)
 	DeleteProjectAccessRequest(ctx context.Context, id string) error
 
+	// FindBookmarks returns the bookmarks in a project that are visible to the user: their own plus shared and default ones.
+	// resourceKind and resourceName are optional filters; when both are empty, all bookmarks in the project are returned.
 	FindBookmarks(ctx context.Context, projectID, resourceKind, resourceName, userID string) ([]*Bookmark, error)
 	FindBookmark(ctx context.Context, bookmarkID string) (*Bookmark, error)
 	FindDefaultBookmark(ctx context.Context, projectID, resourceKind, resourceName string) (*Bookmark, error)

admin/database/postgres/postgres.go

@@ -2852,11 +2852,23 @@ func (c *connection) DeleteProjectAccessRequest(ctx context.Context, id string)
 	return checkDeleteRow("project access request", res, err)
 }
 
-// FindBookmarks returns a list of bookmarks for a user per project
+// FindBookmarks returns the bookmarks in a project that are visible to the user.
+// resourceKind and resourceName are optional filters; when empty, they are not applied.
 func (c *connection) FindBookmarks(ctx context.Context, projectID, resourceKind, resourceName, userID string) ([]*database.Bookmark, error) {
+	qry := `SELECT * FROM bookmarks WHERE project_id = $1 AND (user_id = $2 OR shared = true OR "default" = true)`
+	args := []any{projectID, userID}
+	if resourceKind != "" {
+		args = append(args, resourceKind)
+		qry += fmt.Sprintf(" AND resource_kind = $%d", len(args))
+	}
+	if resourceName != "" {
+		args = append(args, resourceName)
+		qry += fmt.Sprintf(" AND lower(resource_name) = lower($%d)", len(args))
+	}
+	qry += " ORDER BY lower(display_name), created_on"
+
 	var res []*database.Bookmark
-	err := c.getDB(ctx).SelectContext(ctx, &res, `SELECT * FROM bookmarks WHERE project_id = $1 and resource_kind = $2 and lower(resource_name) = lower($3) and (user_id = $4 or shared = true or "default" = true)`,
-		projectID, resourceKind, resourceName, userID)
+	err := c.getDB(ctx).SelectContext(ctx, &res, qry, args...)
 	if err != nil {
 		return nil, parseErr("bookmarks", err)
 	}
@@ -2903,7 +2915,7 @@ func (c *connection) UpdateBookmark(ctx context.Context, opts *database.UpdateBo
 	if err := database.Validate(opts); err != nil {
 		return err
 	}
-	res, err := c.getDB(ctx).ExecContext(ctx, `UPDATE bookmarks SET display_name=$1, description=$2, url_search=$3, shared=$4 WHERE id=$5`,
+	res, err := c.getDB(ctx).ExecContext(ctx, `UPDATE bookmarks SET display_name=$1, description=$2, url_search=$3, shared=$4, updated_on=now() WHERE id=$5`,
 		opts.DisplayName, opts.Description, opts.URLSearch, opts.Shared, opts.BookmarkID)
 	return checkUpdateRow("bookmark", res, err)
 }

admin/server/bookmarks.go

@@ -12,14 +12,34 @@ import (
 	"google.golang.org/protobuf/types/known/timestamppb"
 )
 
-// ListBookmarks server returns the bookmarks for the user per project
+// ListBookmarks returns the bookmarks in a project that are visible to the user: their own plus shared and default ones.
+// The resource kind and name are optional filters; when both are empty, all bookmarks in the project are returned.
 func (s *Server) ListBookmarks(ctx context.Context, req *adminv1.ListBookmarksRequest) (*adminv1.ListBookmarksResponse, error) {
 	claims := auth.GetClaims(ctx)
 	// Error if authenticated as anything other than a user
 	if claims.OwnerType() != auth.OwnerTypeUser {
 		return nil, status.Error(codes.Unauthenticated, "not authenticated as a user")
 	}
 
+	if req.ResourceName != "" && req.ResourceKind == "" {
+		return nil, status.Error(codes.InvalidArgument, "resource_kind is required when resource_name is set")
+	}
+
+	proj, err := s.admin.DB.FindProject(ctx, req.ProjectId)
+	if err != nil {
+		return nil, err
+	}
+
+	permissions := claims.ProjectPermissions(ctx, proj.OrganizationID, proj.ID)
+	if proj.Public {
+		permissions.ReadProject = true
+		permissions.ReadProd = true
+	}
+
+	if !permissions.ReadProject {
+		return nil, status.Error(codes.PermissionDenied, "does not have permission to read the project")
+	}
+
 	bookmarks, err := s.admin.DB.FindBookmarks(ctx, req.ProjectId, req.ResourceKind, req.ResourceName, claims.OwnerID())
 	if err != nil {
 		return nil, err

admin/server/bookmarks_test.go

@@ -0,0 +1,143 @@
+package server_test
+
+import (
+	"context"
+	"testing"
+
+	"github.com/rilldata/rill/admin/database"
+	"github.com/rilldata/rill/admin/testadmin"
+	adminv1 "github.com/rilldata/rill/proto/gen/rill/admin/v1"
+	"github.com/stretchr/testify/require"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
+)
+
+func TestBookmarks(t *testing.T) {
+	ctx := context.Background()
+	fix := testadmin.New(t)
+
+	const exploreKind = "rill.runtime.v1.Explore"
+	const canvasKind = "rill.runtime.v1.Canvas"
+
+	// Create an admin user with an org and a project.
+	_, admin := fix.NewUser(t)
+	org, err := admin.CreateOrganization(ctx, &adminv1.CreateOrganizationRequest{Name: randomName()})
+	require.NoError(t, err)
+	proj, err := admin.CreateProject(ctx, &adminv1.CreateProjectRequest{
+		Org:        org.Organization.Name,
+		Project:    "proj1",
+		ProdSlots:  1,
+		SkipDeploy: true,
+	})
+	require.NoError(t, err)
+	projectID := proj.Project.Id
+
+	// Add a viewer to the project.
+	viewerUser, viewer := fix.NewUser(t)
+	_, err = admin.AddProjectMemberUser(ctx, &adminv1.AddProjectMemberUserRequest{
+		Org:     org.Organization.Name,
+		Project: proj.Project.Name,
+		Email:   viewerUser.Email,
+		Role:    database.ProjectRoleNameViewer,
+	})
+	require.NoError(t, err)
+
+	// A user that is not a member of the project.
+	_, outsider := fix.NewUser(t)
+
+	// Bookmarks across two dashboards: a shared one and a personal one by the admin, and a personal one by the viewer.
+	shared, err := admin.CreateBookmark(ctx, &adminv1.CreateBookmarkRequest{
+		DisplayName:  "Shared explore bookmark",
+		ProjectId:    projectID,
+		ResourceKind: exploreKind,
+		ResourceName: "explore1",
+		Shared:       true,
+		UrlSearch:    "?tr=P7D",
+	})
+	require.NoError(t, err)
+	_, err = admin.CreateBookmark(ctx, &adminv1.CreateBookmarkRequest{
+		DisplayName:  "Admin personal canvas bookmark",
+		ProjectId:    projectID,
+		ResourceKind: canvasKind,
+		ResourceName: "canvas1",
+		UrlSearch:    "?tr=P1D",
+	})
+	require.NoError(t, err)
+	viewerPersonal, err := viewer.CreateBookmark(ctx, &adminv1.CreateBookmarkRequest{
+		DisplayName:  "Viewer personal explore bookmark",
+		ProjectId:    projectID,
+		ResourceKind: exploreKind,
+		ResourceName: "explore1",
+		UrlSearch:    "?tr=P30D",
+	})
+	require.NoError(t, err)
+
+	t.Run("project-wide listing returns own, shared and default bookmarks in a stable order", func(t *testing.T) {
+		res, err := admin.ListBookmarks(ctx, &adminv1.ListBookmarksRequest{ProjectId: projectID})
+		require.NoError(t, err)
+		require.Equal(t, []string{"Admin personal canvas bookmark", "Shared explore bookmark"}, bookmarkNames(res.Bookmarks))
+
+		res, err = viewer.ListBookmarks(ctx, &adminv1.ListBookmarksRequest{ProjectId: projectID})
+		require.NoError(t, err)
+		require.Equal(t, []string{"Shared explore bookmark", "Viewer personal explore bookmark"}, bookmarkNames(res.Bookmarks))
+	})
+
+	t.Run("listing can be filtered by resource", func(t *testing.T) {
+		res, err := admin.ListBookmarks(ctx, &adminv1.ListBookmarksRequest{ProjectId: projectID, ResourceKind: exploreKind, ResourceName: "explore1"})
+		require.NoError(t, err)
+		require.Equal(t, []string{"Shared explore bookmark"}, bookmarkNames(res.Bookmarks))
+
+		// The resource name is matched case-insensitively.
+		res, err = viewer.ListBookmarks(ctx, &adminv1.ListBookmarksRequest{ProjectId: projectID, ResourceKind: exploreKind, ResourceName: "EXPLORE1"})
+		require.NoError(t, err)
+		require.Equal(t, []string{"Shared explore bookmark", "Viewer personal explore bookmark"}, bookmarkNames(res.Bookmarks))
+
+		// Filtering by kind only.
+		res, err = admin.ListBookmarks(ctx, &adminv1.ListBookmarksRequest{ProjectId: projectID, ResourceKind: canvasKind})
+		require.NoError(t, err)
+		require.Equal(t, []string{"Admin personal canvas bookmark"}, bookmarkNames(res.Bookmarks))
+
+		// A resource name without a kind is rejected.
+		_, err = admin.ListBookmarks(ctx, &adminv1.ListBookmarksRequest{ProjectId: projectID, ResourceName: "explore1"})
+		require.Equal(t, codes.InvalidArgument, status.Code(err))
+	})
+
+	t.Run("listing requires read access to the project", func(t *testing.T) {
+		_, err := outsider.ListBookmarks(ctx, &adminv1.ListBookmarksRequest{ProjectId: projectID})
+		require.Equal(t, codes.PermissionDenied, status.Code(err))
+	})
+
+	t.Run("updating a bookmark bumps updated_on", func(t *testing.T) {
+		before, err := viewer.GetBookmark(ctx, &adminv1.GetBookmarkRequest{BookmarkId: viewerPersonal.Bookmark.Id})
+		require.NoError(t, err)
+
+		_, err = viewer.UpdateBookmark(ctx, &adminv1.UpdateBookmarkRequest{
+			BookmarkId:  viewerPersonal.Bookmark.Id,
+			DisplayName: "Viewer personal explore bookmark (renamed)",
+			UrlSearch:   "?tr=P30D",
+		})
+		require.NoError(t, err)
+
+		after, err := viewer.GetBookmark(ctx, &adminv1.GetBookmarkRequest{BookmarkId: viewerPersonal.Bookmark.Id})
+		require.NoError(t, err)
+		require.Equal(t, "Viewer personal explore bookmark (renamed)", after.Bookmark.DisplayName)
+		require.True(t, after.Bookmark.UpdatedOn.AsTime().After(before.Bookmark.UpdatedOn.AsTime()))
+
+		// A viewer cannot update a shared bookmark.
+		_, err = viewer.UpdateBookmark(ctx, &adminv1.UpdateBookmarkRequest{
+			BookmarkId:  shared.Bookmark.Id,
+			DisplayName: "Hijacked",
+			Shared:      true,
+			UrlSearch:   "?tr=P7D",
+		})
+		require.Equal(t, codes.PermissionDenied, status.Code(err))
+	})
+}
+
+func bookmarkNames(bookmarks []*adminv1.Bookmark) []string {
+	names := make([]string, len(bookmarks))
+	for i, b := range bookmarks {
+		names[i] = b.DisplayName
+	}
+	return names
+}

proto/gen/rill/admin/v1/admin.swagger.yaml

@@ -4650,10 +4650,14 @@ paths:
           required: false
           type: string
         - name: resourceKind
+          description: |-
+            Optional filter on the kind of the resource the bookmark is for (e.g. "rill.runtime.v1.Explore").
+            When both resource_kind and resource_name are empty, all bookmarks in the project are returned.
           in: query
           required: false
           type: string
         - name: resourceName
+          description: Optional filter on the name of the resource the bookmark is for. Requires resource_kind to be set.
           in: query
           required: false
           type: string

proto/gen/rill/admin/v1/openapi.yaml

@@ -8153,11 +8153,15 @@ paths:
                   name: projectId
                   schema:
                     type: string
-                - in: query
+                - description: |-
+                    Optional filter on the kind of the resource the bookmark is for (e.g. "rill.runtime.v1.Explore").
+                    When both resource_kind and resource_name are empty, all bookmarks in the project are returned.
+                  in: query
                   name: resourceKind
                   schema:
                     type: string
-                - in: query
+                - description: Optional filter on the name of the resource the bookmark is for. Requires resource_kind to be set.
+                  in: query
                   name: resourceName
                   schema:
                     type: string

proto/rill/admin/v1/api.proto

@@ -2650,9 +2650,14 @@ message RevokeCurrentAuthTokenRequest {}
 message RevokeCurrentAuthTokenResponse {
 }
 
+// ListBookmarksRequest lists the bookmarks in a project that are visible to the caller:
+// the caller's own bookmarks plus shared and default bookmarks.
 message ListBookmarksRequest {
   string project_id = 1;
+  // Optional filter on the kind of the resource the bookmark is for (e.g. "rill.runtime.v1.Explore").
+  // When both resource_kind and resource_name are empty, all bookmarks in the project are returned.
   string resource_kind = 2;
+  // Optional filter on the name of the resource the bookmark is for. Requires resource_kind to be set.
   string resource_name = 3;
 }
 

web-admin/src/client/gen/index.schemas.ts

@@ -2534,7 +2534,14 @@ This is only allowed for superusers. */
 
 export type AdminServiceListBookmarksParams = {
   projectId?: string;
+  /**
+ * Optional filter on the kind of the resource the bookmark is for (e.g. "rill.runtime.v1.Explore").
+When both resource_kind and resource_name are empty, all bookmarks in the project are returned.
+ */
   resourceKind?: string;
+  /**
+   * Optional filter on the name of the resource the bookmark is for. Requires resource_kind to be set.
+   */
   resourceName?: string;
 };