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

webui: Update request IDs when removing mount point row #5077

Merged

Conversation

vojtechtrefny
Copy link
Contributor

When removing a mount point row, we need to decrease IDs for the requests with higher IDs to avoid having duplicate ID when adding a new row.

Resolves: rhbz#2232398

@vojtechtrefny
Copy link
Contributor Author

I am not sure if this is the best way to fix this issue, but it seems to be working fine. The problem is that after removing a row which is not last we end up with a "gap" in the IDs and next row/mount point added now has the same ID as the previous one, because new IDs are allocated simply as number of requests + 1. I was thinking about changing this to last request + 1 but that also might be problematic because we also use the request ID as the row ID. Feel free to suggest a better approach.

Copy link
Member

@jkonecny12 jkonecny12 left a comment

Choose a reason for hiding this comment

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

Looks good to me AFAICT :)

@jkonecny12 jkonecny12 added the f40 label Aug 25, 2023
When removing a mount point row, we need to decrease IDs for the
requests with higher IDs to avoid having duplicate ID when adding
a new row.

Resolves: rhbz#2232398
@vojtechtrefny
Copy link
Contributor Author

/kickstart-test --waive webui-only

Copy link
Contributor

@M4rtinK M4rtinK left a comment

Choose a reason for hiding this comment

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

Looks good to me as well. :)

@vojtechtrefny vojtechtrefny merged commit eda3584 into rhinstaller:master Aug 28, 2023
14 checks passed
Copy link
Contributor

@VladimirSlavik VladimirSlavik left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thank you!

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

Successfully merging this pull request may close these issues.

4 participants