Skip to content

Conversation

@AndyButland
Copy link
Contributor

Prerequisites

  • I have added steps to test this contribution in the description below

Resolves: #13981

Description

This PR provides an update from #14012, which was reverted, and addresses the issues identified in that.

Also fixes an issue where if the same tag is provided twice in a property, differing only by case, it would cause an exception on SQLite.

The integration test added would fail on SQLite (but not LocalDb) before the changes applied in TagRepository. After the changes it passes with both database types.

Testing

For manual testing enter the same tag twice with different casing (you may find you can only do this via the UI within a single property, as the UI will "correct" the casing for tags is already knows about). You should find only one tag written to the cmsTags table when using both SQLite and SQL Server.

Copilot AI review requested due to automatic review settings May 28, 2025 06:32
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

Ensures tag operations insert tags case-insensitively across SQLite and SQL Server and avoids duplicate tags when differing only by case.

  • Added an integration test for mixed-case tag assignments to validate behavior across DBs
  • Updated TagRepository SQL joins to use LOWER() for case-insensitive matching
  • Enhanced in-memory tag comparisons to use case-insensitive logic

Reviewed Changes

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

File Description
tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/TagRepositoryTest.cs Removed using System.Linq; and added new mixed-casing integration test
src/Umbraco.Infrastructure/Persistence/Repositories/Implement/TagRepository.cs Modified SQL joins for case-insensitive tag matching and updated comparer
Comments suppressed due to low confidence (2)

tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/TagRepositoryTest.cs:3

  • The LINQ extension methods .First() and .Count() are used in the new test but using System.Linq; was removed. Please re-add the import so the test compiles.
using System.Linq;

src/Umbraco.Infrastructure/Persistence/Repositories/Implement/TagRepository.cs:251

  • [nitpick] Consider using string.Equals(x.Text, y.Text, StringComparison.OrdinalIgnoreCase) or StringComparer.OrdinalIgnoreCase to avoid allocating new strings via ToLowerInvariant().
(x.Text.ToLowerInvariant() == y.Text.ToLowerInvariant())

@AndyButland AndyButland requested a review from Migaroez May 29, 2025 06:22
@Migaroez Migaroez merged commit ebd228c into v13/dev May 30, 2025
19 checks passed
@Migaroez Migaroez deleted the v13/bugfix/ensure-tags-case-insensitive-on-sql-lite branch May 30, 2025 07:05
AndyButland added a commit that referenced this pull request May 30, 2025
…types (#19439)

* Ensure tag operations are case insensitve on insert across database types.

* Ensure tags provided in a single property are case insensitively distinct when saving the tags and relationships.

* Update src/Umbraco.Infrastructure/Persistence/Repositories/Implement/TagRepository.cs

Co-authored-by: Copilot <[email protected]>

* Handle case sensitivity on insert with tag groups too.

---------

Co-authored-by: Copilot <[email protected]>
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.

3 participants