Skip to content

Conversation

@skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Jul 31, 2025

@skjnldsv skjnldsv added this to the Nextcloud 32 milestone Jul 31, 2025
@skjnldsv skjnldsv self-assigned this Jul 31, 2025
@skjnldsv skjnldsv requested a review from a team as a code owner July 31, 2025 06:57
@skjnldsv skjnldsv requested review from nfebe and removed request for a team July 31, 2025 06:57
@skjnldsv skjnldsv added the bug label Jul 31, 2025
@skjnldsv skjnldsv requested review from a team and kesselb July 31, 2025 06:57
@skjnldsv skjnldsv force-pushed the fix/unique-vcategory branch 2 times, most recently from 2753200 to 49a3794 Compare July 31, 2025 08:48
@skjnldsv skjnldsv requested review from CarlSchwan and come-nc August 5, 2025 12:18
This was referenced Aug 5, 2025
Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

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

Did I miss it or is there no change apart from the unique index?

@skjnldsv skjnldsv force-pushed the fix/unique-vcategory branch from 49a3794 to d1a35bb Compare August 5, 2025 12:45
@skjnldsv skjnldsv requested a review from come-nc August 5, 2025 12:45
@skjnldsv
Copy link
Member Author

skjnldsv commented Aug 5, 2025

Did I miss it or is there no change apart from the unique index?

the removal of duplicates in the migration ?

@skjnldsv skjnldsv force-pushed the fix/unique-vcategory branch from d1a35bb to e78ca06 Compare August 5, 2025 12:46
@come-nc
Copy link
Contributor

come-nc commented Aug 5, 2025

Did I miss it or is there no change apart from the unique index?

the removal of duplicates in the migration ?

But that’s fixing the existing, but how did they get in there?
The code path that previously added them will now blow up with a DB exception, no?

@skjnldsv
Copy link
Member Author

skjnldsv commented Aug 5, 2025

But that’s fixing the existing, but how did they get in there? The code path that previously added them will now blow up with a DB exception, no?

As far as I understood, it's some very edge case where on a race condition, we cache the list of existing tags which means we can allow multiple to be created because the tag existing check won't have the most recent list.

Erroring out is ok from me 🤷
The goal is to fix the existing data and preventing it from happening again.
Allowing to handle favorites again

@skjnldsv
Copy link
Member Author

skjnldsv commented Aug 5, 2025

/backport to stable31

@skjnldsv
Copy link
Member Author

skjnldsv commented Aug 5, 2025

/backport to stable30

Copy link
Contributor

@kesselb kesselb left a comment

Choose a reason for hiding this comment

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

Thanks 🙏

@skjnldsv skjnldsv force-pushed the fix/unique-vcategory branch from e78ca06 to 9aac182 Compare August 5, 2025 20:11
@skjnldsv skjnldsv enabled auto-merge August 6, 2025 05:07
@skjnldsv skjnldsv merged commit 687df7c into master Aug 6, 2025
216 of 220 checks passed
@skjnldsv skjnldsv deleted the fix/unique-vcategory branch August 6, 2025 05:13
@backportbot
Copy link

backportbot bot commented Aug 6, 2025

The backport to stable30 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable30
git pull origin stable30

# Create the new backport branch
git checkout -b backport/54165/stable30

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts, resolve them
git cherry-pick 9aac1821

# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/54165/stable30

Error: Failed to check for changes with origin/stable30: No changes found in backport branch


Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

@backportbot
Copy link

backportbot bot commented Aug 6, 2025

The backport to stable31 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable31
git pull origin stable31

# Create the new backport branch
git checkout -b backport/54165/stable31

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts, resolve them
git cherry-pick 9aac1821

# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/54165/stable31

Error: Failed to check for changes with origin/stable31: No changes found in backport branch


Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

/**
* Make sure vcategory entries are unique per user and type
* This migration will clean up existing duplicates
* and add a unique constraint to prevent future duplicates.
Copy link
Member

Choose a reason for hiding this comment

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

It doesn't?

Copy link
Member Author

Choose a reason for hiding this comment

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

Leftover comment from previous approach.

@skjnldsv skjnldsv modified the milestones: Nextcloud 32, Nextcloud 33 Sep 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Incomplete handling for tags with the same name

5 participants