Conversation
refactor new extensions into another file
…xProvider-methods
…xProvider-methods
…ethods' of https://github.com/idseefeld/Umbraco-CMS into v173/21451-quote-raw-sql-names-with-SqlSyntaxProvider-methods
…n UmbracoDatabaseFactory
…xProvider-methods
…xProvider-methods
…3-remaning-issues
…tabaseDataCreator-refactor
…3-remaning-issues
|
Hi there @idseefeld, thank you for this contribution! 👍 While we wait for one of the Core Collaborators team to have a look at your work, we wanted to let you know about that we have a checklist for some of the things we will consider during review:
Don't worry if you got something wrong. We like to think of a pull request as the start of a conversation, we're happy to provide guidance on improving your contribution. If you realize that you might want to make some changes then you can do that by adding new commits to the branch you created for this work and pushing new commits. They should then automatically show up as updates to this pull request. Thanks, from your friendly Umbraco GitHub bot 🤖 🙂 |
There was a problem hiding this comment.
Pull request overview
This PR aims to improve NPoco compatibility for several persistence DTOs (especially composite-key / non-autoincrement scenarios) and to avoid case-sensitive SQL issues (e.g., PostgreSQL) by relying on DTO column name constants in delete clauses.
Changes:
- Updated repository delete clauses to use DTO column-name constants instead of hard-coded strings.
- Adjusted multiple DTOs’ primary key / table metadata (adding
[PrimaryKey], introducing constants, updating PK column references for deletes). - Refactored some DTO constants to differentiate between key columns vs. other identifiers.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MemberRepository.cs | Uses DTO column constant for nodeId delete clause to improve provider compatibility. |
| src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs | Updates delete clauses to target the correct key columns after DTO constant refactors. |
| src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentTypeRepository.cs | Updates delete clauses to use updated DTO key column constants. |
| src/Umbraco.Infrastructure/Persistence/Dtos/Webhook2HeadersDto.cs | Adds class-level PK metadata and refactors PK naming/constants. |
| src/Umbraco.Infrastructure/Persistence/Dtos/Webhook2EventsDto.cs | Adds class-level PK metadata and refactors PK naming/constants. |
| src/Umbraco.Infrastructure/Persistence/Dtos/Webhook2ContentTypeKeysDto.cs | Adds class-level PK metadata and refactors PK naming/constants. |
| src/Umbraco.Infrastructure/Persistence/Dtos/UserGroup2NodePermissionDto.cs | Adds class-level PK metadata and refactors PK naming/constants for composite PK. |
| src/Umbraco.Infrastructure/Persistence/Dtos/UserGroup2NodeDto.cs | Adds class-level PK metadata and refactors PK naming/constants for composite PK. |
| src/Umbraco.Infrastructure/Persistence/Dtos/UserGroup2LanguageDto.cs | Adds class-level PK metadata and refactors PK naming/constants for composite PK. |
| src/Umbraco.Infrastructure/Persistence/Dtos/UserGroup2AppDto.cs | Refactors constants so PK constraint name and key columns are clearer (but see review comments). |
| src/Umbraco.Infrastructure/Persistence/Dtos/User2UserGroupDto.cs | Adds class-level PK metadata and refactors PK naming/constants for composite PK. |
| src/Umbraco.Infrastructure/Persistence/Dtos/User2NodeNotifyDto.cs | Refactors constants for composite PK and updates repositories to use the correct column constants. |
| src/Umbraco.Infrastructure/Persistence/Dtos/User2ClientIdDto.cs | Refactors constants for composite PK and used by UserRepository insert/delete flows. |
| src/Umbraco.Infrastructure/Persistence/Dtos/TagRelationshipDto.cs | Refactors constants and index definition to reference the correct nodeId column constant. |
| src/Umbraco.Infrastructure/Persistence/Dtos/Member2MemberGroupDto.cs | Refactors constants for composite PK. |
| src/Umbraco.Infrastructure/Persistence/Dtos/ContentVersionCleanupPolicyDto.cs | Renames PK constant usage (column name) and aligns PK constraint naming. |
| src/Umbraco.Infrastructure/Persistence/Dtos/ContentTypeTemplateDto.cs | Refactors constants for composite PK and column naming. |
| src/Umbraco.Infrastructure/Persistence/Dtos/ContentTypeAllowedContentTypeDto.cs | Refactors PK-related constants and FK column references. |
| src/Umbraco.Infrastructure/Persistence/Dtos/ContentType2ContentTypeDto.cs | Refactors constants for composite PK and column naming. |
| src/Umbraco.Infrastructure/Persistence/Dtos/ContentNuDto.cs | Refactors composite PK metadata and FK/index column references. |
src/Umbraco.Infrastructure/Persistence/Dtos/ContentType2ContentTypeDto.cs
Show resolved
Hide resolved
src/Umbraco.Infrastructure/Persistence/Dtos/Webhook2ContentTypeKeysDto.cs
Outdated
Show resolved
Hide resolved
…rectly, a single test would reveal the problem.
This is another PR for issue #21675.
For DTOs without autoIncrement columns and multi-column primary keys attributes need updates to be fully compatible with NPoco. Especially an PostgreSQL provider has otherwise issues.
The created table sql stays unchanged!
All test must succeed and must not be any changes for existing database providers.