Skip to content

Conversation

@hifi-phil
Copy link
Contributor

Description

Some Claude memory files that have been generated against a template to attempt to standardise memory file product for Umbraco

Add comprehensive CLAUDE.md files for major Umbraco projects:

  • Root CLAUDE.md: Multi-project repository overview
  • Umbraco.Core: Interface contracts and domain models
  • Umbraco.Infrastructure: Implementation layer (NPoco, migrations, services)
  • Umbraco.Cms.Api.Common: Shared API infrastructure
  • Umbraco.Cms.Api.Management: Management API (1,317 files, 54 domains)
  • Umbraco.Web.UI.Client: Frontend with split docs structure

Each file includes:

  • Architecture and design patterns
  • Project-specific workflows
  • Edge cases and gotchas
  • Commands and setup
  • Technical debt tracking

Add comprehensive CLAUDE.md files for major Umbraco projects:
- Root CLAUDE.md: Multi-project repository overview
- Umbraco.Core: Interface contracts and domain models
- Umbraco.Infrastructure: Implementation layer (NPoco, migrations, services)
- Umbraco.Cms.Api.Common: Shared API infrastructure
- Umbraco.Cms.Api.Management: Management API (1,317 files, 54 domains)
- Umbraco.Web.UI.Client: Frontend with split docs structure

Each file includes:
- Architecture and design patterns
- Project-specific workflows
- Edge cases and gotchas
- Commands and setup
- Technical debt tracking

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
Copy link
Contributor

@AndyButland AndyButland left a comment

Choose a reason for hiding this comment

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

Seems pretty good to me @hifi-phil. I've given just a cursory review to check it's on the right track and added a few comments where I saw errors.

CLAUDE.md Outdated
@@ -0,0 +1,339 @@
# Umbraco CMS - Multi-Project Repository

Enterprise-grade headless CMS built on .NET 10.0. This repository contains 21 production projects organized in a layered architecture with clear separation of concerns.
Copy link
Contributor

Choose a reason for hiding this comment

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

Not (just) headless.

CLAUDE.md Outdated
Comment on lines 146 to 147
- Features: `feature/description` or `contrib/username/description`
- Bug fixes: `bugfix/description` or `fix/issue-number`
Copy link
Contributor

Choose a reason for hiding this comment

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


### Pull Request Process

- **PR Template**: `.github/pull_request_template.md`
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

does this exist in the repo somewhere?

CLAUDE.md Outdated
### Migration from Legacy to EF Core

The repository contains BOTH:
- **Legacy**: NPoco-based persistence (`Umbraco.Cms.Persistence.Sqlite`, `Umbraco.Cms.Persistence.SqlServer`)
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't say this is legacy, at least yet.

CLAUDE.md Outdated
- **Legacy**: NPoco-based persistence (`Umbraco.Cms.Persistence.Sqlite`, `Umbraco.Cms.Persistence.SqlServer`)
- **Modern**: EF Core-based persistence (`Umbraco.Cms.Persistence.EFCore.*`)

**Recommendation**: New development should use EF Core projects.
Copy link
Contributor

Choose a reason for hiding this comment

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

Not correct (yet).

### Authentication: OpenIddict

All APIs use **OpenIddict** (OAuth 2.0/OpenID Connect):
- Reference tokens (not JWT) for better security
Copy link
Contributor

Choose a reason for hiding this comment

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

More to say on this now - see #20820

CLAUDE.md Outdated

1. **Circular Dependencies**: Avoided via `Lazy<T>` or event notifications
2. **Multi-Server**: Requires shared Data Protection key ring and synchronized clocks (NTP)
3. **Database Support**: SQL Server, SQLite (MySQL/PostgreSQL via community packages)
Copy link
Contributor

Choose a reason for hiding this comment

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

MySQL/PostgreSQL via community packages - not (yet) true.

Comment on lines 337 to 340
### Data Access Security
**SQL Injection Prevention**:
- All data access via EF Core (parameterized queries)
- No raw SQL in this project
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be NPoco not EF Core, but not really relevant in this project anyway.

@AndyButland AndyButland requested a review from Migaroez November 15, 2025 11:55
Copy link
Contributor

@iOvergaard iOvergaard left a comment

Choose a reason for hiding this comment

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

Added a few suggestions around the Client. In general, some of the agentic actions are over-simplified, but I'm not sure if the agent can figure it out by looking at the existing code anyway.

In the future, we might have use of more flows, such as:

  • Adding a dashboard
  • Adding a workspace
  • Using the existing Content workspace

Comment on lines +121 to +125
The project embeds a pre-generated `OpenApi.json` (1.3MB). To regenerate:
```bash
# Run Umbraco.Web.UI, access /umbraco/swagger
# Export JSON from Swagger UI
```
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps it would help to add the URL to the raw schema, which we (manually) copy over to the OpenAPI.json file: /umbraco/swagger/management/swagger.json


### Known Limitations

1. **API Versioning**: Currently only v1.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Some resources are already on V1.1. Well, 1 operation is. See ValidateUpdateDocumentController#ValidateV1_1.

3. **Real-time Limits**: SignalR hubs don't scale beyond single-server without Redis backplane
- Configure Redis for multi-server setups

4. **File Upload Size**: Controlled by parent app (Umbraco.Web.UI)
Copy link
Contributor

Choose a reason for hiding this comment

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

I see Umbraco.Web.UI was mentioned several times. The Umbraco.Web.UI project only controls these values in development mode. In a real Umbraco project, all settings are generated through the appsettings.template.json files through the UmbracoTemplate dotnet template.


### Phase 2: Incremental Implementation

**For New Feature (Component/Package)**:
Copy link
Contributor

Choose a reason for hiding this comment

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

The interfaces described below only apply to new extensions, and not necessarily all new features.

```typescript
// 6. Connect user interactions to logic
@customElement('umb-my-element')
export class UmbMyElement extends UmbElementMixin(LitElement) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
export class UmbMyElement extends UmbElementMixin(LitElement) {
export class UmbMyElement extends UmbLitElement {

Comment on lines +140 to +154
async #handleClick() {
const { data, error } = await this.#context?.load();
if (error) {
this._error = error;
return;
}
this._data = data;
}

render() {
return html`
<button @click=${this.#handleClick}>Load</button>
${this._data ? html`<p>${this._data.name}</p>` : ''}
`;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
async #handleClick() {
const { data, error } = await this.#context?.load();
if (error) {
this._error = error;
return;
}
this._data = data;
}
render() {
return html`
<button @click=${this.#handleClick}>Load</button>
${this._data ? html`<p>${this._data.name}</p>` : ''}
`;
}
@state()
private _data?: UmbMyModel;
async #handleClick() {
const { data, error } = await this.#context?.load();
if (error) {
this._error = error;
return;
}
this._data = data;
}
render() {
return html`
<uui-button @click=${this.#handleClick} label="Load"></uui-button>
${this._data ? html`<p>${this._data.name}</p>` : nothing}
`;
}


render() {
return html`
<button @click=${this.#handleClick}>Load</button>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<button @click=${this.#handleClick}>Load</button>
<uui-button @click=${this.#handleClick} label="Load"></uui-button>

}
}
```

Copy link
Contributor

@iOvergaard iOvergaard Nov 17, 2025

Choose a reason for hiding this comment

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

Suggested change
**Step 9: Add localization**
```typescript
// 10. Add to src/Umbraco.Web.UI.Client/src/assets/lang/en.ts and other appropriate files
{
actions: {
load: 'Load'
}
}
// 11. Use the localize helper (`this.localize.term()`) in the element
render() {
return html`
<uui-button @click=${this.#handleClick} label=${this.localize.term('actions_load')></uui-button>
${this._data ? html`<p>${this._data.name}</p>` : ''}
`;
}
async #handleClick() {
try {
this._loading = true;
const { data, error } = await this.#context?.load();
if (error) {
this._error = this.localize.term('errors_receivedErrorFromServer');
return;
}
this._data = data;
} catch (error) {
this._error = this.localize.term('errors_defaultError');
console.error('Load failed:', error);
} finally {
this._loading = false;
}
}
// 12. Outside elements (such as controllers), use the Localization Controller
export class UmbMyController extends UmbControllerBase {
#localize = new UmbLocalizationController(this);
#notificationContext?: typeof UMB_NOTIFICATION_CONTEXT.TYPE;
constructor(host: UmbControllerHost) {
super(host);
this.consumeContext(UMB_NOTIFICATION_CONTEXT, (notificationContext) => {
this.#notificationContext = notificationContext;
});
}
fetchData() {
// Show error
this.#notificationContext?.peek('positive', {
data: {
headline: this.#localize.term('speechBubbles_onlineHeadline'),
message: this.#localize.term('speechBubbles_onlineMessage'),
},
});
}
}

@hifi-phil
Copy link
Contributor Author

In the future, we might have use of more flows, such as:

  • Adding a dashboard
  • Adding a workspace
  • Using the existing Content workspace

I have a plan to provide this type of knowledge using Claude Code Skills. This will allow us to supply much more specific knowledge including examples, guides, etc in a context efficient way.

hifi-phil and others added 4 commits November 17, 2025 12:00
Co-authored-by: Andy Butland <[email protected]>
Co-authored-by: Jacob Overgaard <[email protected]>
- Clarify NPoco is current and fully supported (not legacy)
- Document EF Core as future direction with ongoing migration
- Add secure cookie-based token storage details for v17+
- Update OpenIddict authentication documentation
- Update API versioning (v1.0 and v1.1)
- Minor documentation cleanups (community links, descriptions)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
Copy link
Contributor

@Migaroez Migaroez left a comment

Choose a reason for hiding this comment

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

I will make the proposed changes myself now that I have access to your fork.
Other things todo:

  • check patch strategy with Kenn
  • Cleanup double info in project files that are already the root file

3. **Notification Pattern** (not C# events)
- `*SavingNotification` (before, cancellable)
- `*SavedNotification` (after, for reactions)
- Registered via Composers
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Registered via Composers
- Handlers registered by extention method `AddNotificationHandler`


5. **Scoping Pattern** (Unit of Work)
- `ICoreScopeProvider.CreateCoreScope()`
- Must call `scope.Complete()` to commit
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Must call `scope.Complete()` to commit
- Must call `scope.Complete()` to commit or set the parameter autocomplete to true when calling `ICoreScopeProvider.CreateCoreScope()`, this will complete the scope upon disposal

- **Validation**: FluentValidation via base controllers
- **Serialization**: System.Text.Json with custom converters
- **Mapping**: Manual presentation factories (no AutoMapper)
- **Patching**: JsonPatch.Net for PATCH operations
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- **Patching**: JsonPatch.Net for PATCH operations

Not used as far as I know and can see, and agreed in discussions that this should be avoided at all cost (might be used be an in house) package? @AndyButland @kjac

### Design Patterns
1. **Controller-per-Operation** - Each endpoint is a separate controller class
- Example: `CreateDocumentController`, `UpdateDocumentController`, `DeleteDocumentController`
- Enables fine-grained authorization and operation-specific logic
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Enables fine-grained authorization and operation-specific logic
- Enables fine-grained authorization and operation-specific logic
- **Responsibilities**: entrypoint/routing, authorization and mapping
- **avoid**: business logic directly in controllers (there are a few known violations)

Comment on lines +260 to +264
**Why JsonPatch.Net?**
- RFC 6902 compliant JSON Patch support
- Used for partial updates (PATCH operations)
- See `ViewModels/JsonPatch/JsonPatchViewModel.cs`

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
**Why JsonPatch.Net?**
- RFC 6902 compliant JSON Patch support
- Used for partial updates (PATCH operations)
- See `ViewModels/JsonPatch/JsonPatchViewModel.cs`

Comment on lines +298 to +309
**Authorization**:
```csharp
// Example from CreateDocumentController.cs:22
private readonly IAuthorizationService _authorizationService;

// Authorization checked in base controller methods, not attributes
protected async Task<IActionResult> HandleRequest(request, Func<Task<IActionResult>> handler)
{
var authResult = await _authorizationService.AuthorizeAsync(User, request, policy);
// ...
}
```
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
**Authorization**:
```csharp
// Example from CreateDocumentController.cs:22
private readonly IAuthorizationService _authorizationService;
// Authorization checked in base controller methods, not attributes
protected async Task<IActionResult> HandleRequest(request, Func<Task<IActionResult>> handler)
{
var authResult = await _authorizationService.AuthorizeAsync(User, request, policy);
// ...
}
```
**Authorization**:
- Basic authorization is done trough policies and the `AuthorizeAttribute`
```csharp
// Example from DocumentTreeControllerBase.cs, the user needs at least access to a section that uses trees
[Authorize(Policy = AuthorizationPolicies.SectionAccessForContentTree)]
  • Authorization that needs (parts of) the payload are done manually trough the IAuthorizationService
// Example from CreateDocumentController.cs:22
private readonly IAuthorizationService _authorizationService;

// Authorization checked in base controller methods, not attributes
protected async Task<IActionResult> HandleRequest(request, Func<Task<IActionResult>> handler)
{
    var authResult = await _authorizationService.AuthorizeAsync(User, request, policy);
    // ...
}

- Interfaces in Umbraco.Core, implementations in Umbraco.Infrastructure
- Use `Attempt<TResult, TStatus>` for operations that can fail with specific reasons
- OperationStatus enums provide detailed failure reasons
- Services are registered in Composers via DI
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Services are registered in Composers via DI
- Services are registered via DI in builder extensions

}
}

// 3. Register in composer
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// 3. Register in composer
// 3. Add to builder extensions


### 3. Composer Pattern (DI Registration)

Composers register services and configure the application:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Composers register services and configure the application:
Composers register services and configure the application, this is to make the system easily extendible by package developers and implementors or internally for temporary service registration for use in short lived code, for example migrations :

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.

4 participants