Skip to content

Conversation

@frankiejol
Copy link
Member

Some volume operations go to wrong item. Sort it properly.

@frankiejol frankiejol added this to the v2.4.4 milestone Feb 6, 2026
Copilot AI review requested due to automatic review settings February 6, 2026 09:20
@frankiejol frankiejol closed this Feb 6, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a volume ordering bug where volume operations were going to the wrong volumes due to incorrect sorting. The fix changes the ORDER BY clause in volume queries from sorting by id to sorting by n_order, which is the proper semantic ordering field used throughout the codebase.

Changes:

  • Fixed volume ordering in Ravada::Front::Domain->list_volumes() to use n_order instead of id
  • Added comprehensive test test_order() to verify volumes are returned in correct order even when IDs are non-sequential

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
lib/Ravada/Front/Domain.pm Fixed ORDER BY clause in list_volumes query to use n_order instead of id
t/vm/40_volumes.t Added test_order() function and _swap_volumes_id() helper to verify fix by manipulating volume IDs and checking correct ordering

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +771 to +772
$sth->execute($domain->id);

Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

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

This statement re-executes the query but doesn't fetch or use the results. This line appears to be dead code and can be removed.

Suggested change
$sth->execute($domain->id);

Copilot uses AI. Check for mistakes.
@frankiejol frankiejol deleted the fix/2285_vol branch February 6, 2026 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant