Skip to content

fix: adjust operations api usage WD-35708#1927

Open
omarelkashef wants to merge 1 commit intocanonical:mainfrom
omarelkashef:WD-35708-fix-operations-api-usage
Open

fix: adjust operations api usage WD-35708#1927
omarelkashef wants to merge 1 commit intocanonical:mainfrom
omarelkashef:WD-35708-fix-operations-api-usage

Conversation

@omarelkashef
Copy link
Copy Markdown
Contributor

@omarelkashef omarelkashef commented Apr 7, 2026

Done

QA

  1. Run the LXD-UI:
  2. Perform the following QA steps:
    • Make sure operations such as renaming instance, deleting instance, starting instance from snapshot, creating volume snapshot work, etc. correctly and the notifications are accurate

@webteam-app
Copy link
Copy Markdown

webteam-app commented Apr 7, 2026

Demo

Jenkins

demos.haus

kimanhou
kimanhou reviewed Apr 7, 2026
View reviewed changes
kimanhou
kimanhou reviewed Apr 7, 2026
View reviewed changes
kimanhou
kimanhou reviewed Apr 7, 2026
View reviewed changes
@omarelkashef omarelkashef force-pushed the WD-35708-fix-operations-api-usage branch 3 times, most recently from e492368 to bf48061 Compare April 8, 2026 09:26
Kxiru
Kxiru previously approved these changes Apr 8, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@Kxiru Kxiru left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for addressing this so quickly! I understand this is waiting on another PR?

edlerd
edlerd reviewed Apr 8, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@edlerd edlerd left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the prompt fix here! Small suggestion below.

@omarelkashef omarelkashef force-pushed the WD-35708-fix-operations-api-usage branch from bf48061 to cc6763b Compare April 8, 2026 14:03
edlerd
edlerd previously approved these changes Apr 8, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@edlerd edlerd left a comment

Choose a reason for hiding this comment

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

LGTM

@omarelkashef omarelkashef force-pushed the WD-35708-fix-operations-api-usage branch from cc6763b to f2e742f Compare April 10, 2026 09:22
@kimanhou
Copy link
Copy Markdown
Contributor

kimanhou commented Apr 10, 2026

I assume this failed test is linked to Din's changes on network forward API endpoints. Should be fixed next pulse.

image

I expect other tests to fail (related to storage buckets, network forwards and network peers)

@omarelkashef
Copy link
Copy Markdown
Contributor Author

omarelkashef commented Apr 10, 2026 •
edited
Loading

I assume this failed test is linked to Din's changes on network forward API endpoints. Should be fixed next pulse.

image I expect other tests to fail (related to storage buckets, network forwards and network peers)

It makes sense they related. We will fix the CI one test at a time.

@kimanhou
Copy link
Copy Markdown
Contributor

kimanhou commented Apr 10, 2026

Create network forward will be fixed next pulse.
@omarelkashef Any idea about the other two tests ?
image

@edlerd
Copy link
Copy Markdown
Collaborator

edlerd commented Apr 10, 2026

Create network forward will be fixed next pulse. @omarelkashef Any idea about the other two tests ?

LXD latest/edge is not yet including the operation API fixes from canonical/lxd#18033

Let's re-run CI on this PR to ensure those are fixed once the changes are built into the edge channel.

@omarelkashef
Copy link
Copy Markdown
Contributor Author

omarelkashef commented Apr 10, 2026 •
edited
Loading

LXD latest/edge is not yet including the operation API fixes from canonical/lxd#18033

Changes have not landed in latest/edge yet, but we are rebasing an pushing to be ready.

@omarelkashef omarelkashef force-pushed the WD-35708-fix-operations-api-usage branch from f2e742f to 58c6448 Compare April 13, 2026 09:40
@omarelkashef
fix: adjust operations api usage WD-35708
a8eb9f8 
Signed-off-by: Omar Elkashef <omarelkashef01@gmail.com>
@omarelkashef omarelkashef force-pushed the WD-35708-fix-operations-api-usage branch from 58c6448 to a8eb9f8 Compare April 13, 2026 10:06
edlerd
edlerd reviewed Apr 13, 2026
View reviewed changes
src/util/operations.tsx
Comment on lines +13 to +15
const entityUrlField = isRenamingOperation(operation)
? "original_entity_url"
: "entity_url";
Copy link
Copy Markdown
Collaborator

@edlerd edlerd Apr 13, 2026

Choose a reason for hiding this comment

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

We need to stay backward compatible here. I think this only works with the new schema.

Copy link
Copy Markdown
Contributor Author

@omarelkashef omarelkashef Apr 13, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

In the old schema, original_entity_url is empty so isRenamingOperation returns false and we proceed as normal in getOperationEntityUrls

edlerd
edlerd reviewed Apr 13, 2026
View reviewed changes
src/util/operations.spec.ts
Copy link
Copy Markdown
Collaborator

@edlerd edlerd Apr 13, 2026

Choose a reason for hiding this comment

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

Can we keep the tests to ensure backward compatibility and add new tests for the new schema?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@edlerd edlerd edlerd left review comments

@kimanhou kimanhou kimanhou left review comments

@Kxiru Kxiru Kxiru left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@omarelkashef @webteam-app @kimanhou @edlerd @Kxiru