Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ui): delete objects from the ui #3261

Merged
merged 38 commits into from
Jan 10, 2025

Conversation

gtarpenning
Copy link
Member

@gtarpenning gtarpenning commented Dec 16, 2024

Description

WB-22373

This pr adds support for handling deleted objects in the UI. Specifically:

  • add the ability to delete individual objects and ops from the object/op page
  • add the ability to delete multiple object versions by checkbox selection on the object versions page
  • all deletion from the UI is gated behind being an admin
  • add the styling for viewing deleted objects in the trace table and objects viewer as strikedthrough
  • add support for deleted models in the eval compare page

Testing

Screenshot 2025-01-09 at 2 46 32 PM

test-delete

Rendering a compare-eval page with a deleted model:
Screenshot 2025-01-09 at 2 40 17 PM

@gtarpenning gtarpenning marked this pull request as ready for review December 17, 2024 00:14
@gtarpenning gtarpenning requested review from a team as code owners December 17, 2024 00:14
req: TraceObjDeleteReq
): Promise<void | {detail: string}> {
const res = super.objectDelete(req).then(r => {
if (r && 'detail' in r) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why throw only if there are details?

@@ -25,7 +25,7 @@ export type EvaluationComparisonSummary = {

// Models are the Weave Objects used to define the model logic and properties.
models: {
[modelRef: string]: ModelObj;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

models can now be deleted!

Comment on lines 46 to -50
const evaluationCall = props.state.summary.evaluationCalls[props.callId];
const modelObj = props.state.summary.models[evaluationCall.modelRef];
const objRef = useMemo(
() => parseRef(modelObj.ref) as WeaveObjectRef,
[modelObj.ref]
);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tssweeney does this make sense? I think we actually never needed modelObj because we are only using it for its ref? Which I think is = to evaluationCall.modelRef but could be wrong...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks correct!

@gtarpenning gtarpenning changed the title chore(ui): add frontend handling for deleted objects feat(ui): add frontend handling for deleted objects Jan 10, 2025
@gtarpenning gtarpenning changed the title feat(ui): add frontend handling for deleted objects feat(ui): delete objects from the ui Jan 10, 2025
@gtarpenning gtarpenning requested a review from tssweeney January 10, 2025 00:50
@gtarpenning gtarpenning enabled auto-merge (squash) January 10, 2025 19:16
@gtarpenning gtarpenning disabled auto-merge January 10, 2025 19:16
@gtarpenning gtarpenning merged commit ea3a2d5 into master Jan 10, 2025
121 checks passed
@gtarpenning gtarpenning deleted the griffin/frontend-objects-deletino branch January 10, 2025 21:43
@github-actions github-actions bot locked and limited conversation to collaborators Jan 10, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants