monitors: show every users' code monitors to admins#54981
Conversation
|
Should the Code Monitor owner be updated as well to display the actual owner, and not the signed in user, when viewing the code monitor? https://sourcegraph.com/github.com/sourcegraph/sourcegraph/-/blob/client/web/src/enterprise/code-monitoring/components/CodeMonitorForm.tsx?L184-195 Think it can maybe still stay disabled, but at least show the correct owner |
7a878bf to
853647d
Compare
eseliger
left a comment
There was a problem hiding this comment.
Great full stack work! I left a few inline comments and suggestions on placement and API surface, LMK what you think!
| id: id!, | ||
| update: { | ||
| namespace: authenticatedUser.id, | ||
| namespace: codeMonitor.owner.id, |
There was a problem hiding this comment.
we don't initialize the above owner to the authenticatedUser, does this page still use the correct namespace?
There was a problem hiding this comment.
ManageCodeMonitorPage is the edit page for an already created monitor, so the owner would already be set (and is immutable). Does that answer it, im not sure if i understood completely
| <div className="row mb-5"> | ||
| <div className="d-flex flex-column w-100 col"> | ||
| {authenticatedUser?.siteAdmin && ( | ||
| <> |
There was a problem hiding this comment.
I wonder if this list would be better suited in the site-admin menu, thus not cluttering this list view too much for site-admins that use code monitors for actual usage vs them wearing their site-admin hat and wanting to look at all entries.
There was a problem hiding this comment.
Its a fairly uncluttered page as-is imo, so I dont think its problematic to list them here as well (I'm personally a fan of having things colocated instead of having to go through extra hoops after realizing "oh theyre not listed where I thought they might be").
Not sure under which site-admin section they should go anyway 😅
432fc8b to
a4a9547
Compare
Closes https://github.com/sourcegraph/sourcegraph/issues/50898 <details> <summary>Screenshot from site-admin user view</summary>  </details> ## Test plan Added storybook test, unit tests & manual local testing
Closes https://github.com/sourcegraph/sourcegraph/issues/50898
Screenshot from site-admin user view
Test plan
Added storybook test & manual local testing