GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — feat(site): manage project access from the project details panel by jaaydenh · Pull Request #31246 · coder/coder Conversation
|
| Manage access (owner) | Phone layout |
|---|---|
![]() |
![]() |
Stack (merge bottom first):
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…s review findings
…ss fixture and clarify the empty role
…s UAT findings Sort users and groups by name so the avatar row and dialog keep a stable order, cap tooltip width on narrow screens, hide ErrorAlert debug details, fill the row when exactly eight entries fit, give the +K tile a tooltip, make avatar tiles keyboard focusable, and refetch access when the dialog opens.
…access/manage-access
…ess/members-summary
…access/manage-access # Conflicts: # site/src/pages/AgentsPage/components/ProjectPage/ProjectAccessDialog.tsx # site/src/pages/AgentsPage/components/ProjectPage/ProjectDetailsPanel.test.tsx # site/src/pages/AgentsPage/components/ProjectPage/ProjectDetailsPanel.tsx
…access/manage-access
|
@codex review |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Here are some automated review suggestions for this pull request.
Reviewed commit: 59ed106b62
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
…access/manage-access # Conflicts: # site/src/pages/AgentsPage/components/ProjectPage/ProjectAccessDialog.tsx # site/src/pages/AgentsPage/components/ProjectPage/projectAccess.ts
|
@codex review |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Here are some automated review suggestions for this pull request.
Reviewed commit: a86e682337
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
…access/manage-access # Conflicts: # site/src/pages/AgentsPage/components/ProjectPage/ProjectDetailsPanelView.tsx
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Adds `docs/ai-coder/agents/projects.md`, a short page for Coder Agents
projects, linked under Coder Agents as early access.
The page covers:
- What a project provides.
- Ownership: organization-scoped, private to the creator, 100 per user,
audited.
- Creating, editing, and deleting projects.
- Adding chats to a project: project page, composer picker, **Move to
project**, drag onto a folder, and `PATCH /api/v2/chats/{chat}`.
- Instructions, with a link to the existing Project skills section for
skills.
- Memory: tools, behavior, limits, and the API.
It also links `extending-agents.md#project-skills` to the new page.
The page documents projects as early access with no experiment flag.
Merge it only once `chat-projects` no longer gates the feature. The
memory and skill APIs it mentions still live under `/api/experimental`.
Project sharing is left out until its UI lands (#31246, #31245).
Open PRs that will make parts of this page stale:
- #30683 changes project deletion.
- #30754, #30756, and #30898 add project templates.
> Generated by Coder Agents on behalf of @f0ssel.
|
|
||
| const runMutation = (mutate: (onSettled: () => void) => void) => { | ||
| if (isMutationInFlight.current) { | ||
| const runMutation = ( |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need runMutation and the in-flight refs. Can we drop them and just rely on isPending disabling the controls?
I removed the guard and ran ProjectDetailsPanel.test.tsx. Only the two double-click tests that use userEvent.setup({ delay: null }) failed. When I gave those tests a delay of 0, 1, 16 or 50ms between clicks, they sent one request every time. All the access tests passed without the guard.
So the guard only matters when both clicks fire in the same JS task. A real double click doesn't do that. Even a setTimeout(0) gap is enough for isPending to flip and disable the button.
Keeping it also brings in the guarded resets (resetAccessErrors, onResetErrors), which are only needed because resetting a pending mutation drops its onSettled. It also isn't tested for access changes. And if a double submit ever did slip through, it would just send the same request twice.
At minimum, I'd rather not make it generic for access in this PR.
Generated by Coder Agents on behalf of @DanielleMaywood.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
removed runMutation
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| // Returns focus to the add row after saving a change drops it. | ||
| const focusById = (id: string) => document.getElementById(id)?.focus(); |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
uhhhhh.... what?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
fixed
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| export const ProjectAccessDialog: React.FC<ProjectAccessDialogProps> = ( | ||
| props, | ||
| ) => <ProjectAccessDialogContent {...props} addTriggerId={useId()} />; |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
This wrapper only exists to call useId(), and it calls it inside JSX props, which is easy to miss. It also adds a second component and a second props type.
Can we give ProjectAccessDialog a block body and call useId() at the top instead? The PR description says this was left out to avoid re-indenting ~80 lines. I don't think that's worth the extra layer, and a re-indent is easy to review with whitespace hidden.
Generated by Coder Agents on behalf of @DanielleMaywood.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
simplified so addTriggerId={useId() isnt needed anymore
Sorry, something went wrong.
There was an error while loading. Please reload this page.
The chat project ACL response now includes the project's owner, so the
project page can show who owns a project to everyone who can read it.
`GET
/api/experimental/organizations/{organization}/chats/projects/{project}/acl`
returns a new `owner` field (`MinimalUser`). The owner is never in
`users`, and `GET /api/v2/users/{id}` returns 404 to regular
organization members, so project members and project admins currently
see the owner as "Unknown". The handler loads the owner with a
system-restricted context after the existing read and sharing-disabled
checks, matching how it already loads shared users and groups.
`TestChatProjectSharing/User` and `/Admin` now check the owner as seen
by the owner, a `use` sharee and an `admin` sharee, and that both
sharees still get 404 from the users endpoint. Dogfood UAT on a dev
deployment confirmed the field for all three callers, no owner data for
callers without access (404), and the unchanged 403 when chat sharing is
disabled.
Stack (merge bottom first):
- 👉 this PR
- #31245
- #31246
- #31247
Adds a members section to the chat project Details panel and a read-only "View access" dialog. - The Details card gets a members section at the bottom: "N members[, G groups]" (the owner plus directly shared users; groups are counted separately), a row of avatars with name tooltips, and a "View access" button. The row shows as many avatars as fit the card's width (up to eight) and ends with a "+K" tile for the rest; its tooltip lists up to ten hidden names and "and N more". Member tooltips let clicks through, so they never block the button. - The dialog lists the owner first with an Owner badge, then shared users and groups (sorted naturally by name) with their role. The `use` role shows as "Member" and `admin` as "Admin". A footnote explains that chats in the project start private. - The owner's name and avatar come from the new `owner` field on the ACL response (#31244), so project members and admins see the real owner in the "Project owner" row too, instead of "Unknown". - The section is hidden when chat sharing is disabled (the ACL endpoint returns 403). Loading shows skeletons; other errors show an alert with Retry in both the panel and the dialog. - Adds `getChatProjectACL` to the API client and a `chatProjectACL` query. Editing access comes in #31246. Dogfood UAT passed on a dev deployment: owner, `use` and `admin` viewers all see the owner and the same members list with no users requests, counts and "+K" overflow are correct, the row fits the card from 320 to 1440px with the sidebar open or collapsed, tooltips never swallow the button click, the section hides with `--disable-chat-sharing`, and ACL errors recover with Retry. | Members section (member's view) | View access dialog | | --- | --- | | <img alt="Details panel with the members section, as a project member" src="https://raw.githubusercontent.com/coder/coder/assets/pr-31245-project-members/pr/members-section.png" width="600" /> | <img alt="View access dialog listing owner, admin, member and Everyone" src="https://raw.githubusercontent.com/coder/coder/assets/pr-31245-project-members/pr/view-access-dialog.png" width="600" /> | Stack (merge bottom first): - #31244 - 👉 this PR - #31246 - #31247
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.