Feat/50/manage interesting facts

[emil720a1]

dev

JIRA

• Issue #50: Admin/Interesting facts block

Code reviewers

• @Exlizardium
• @Loki22978964
• @M1R4MI

Second Level Review

• @DrFaust555

Summary of issue

The backend did not fully support administration of the Interesting Facts block. Administrators needed API operations for creating, updating, deleting, retrieving, and reordering facts while preserving validation rules and a stable display order.

The API also needed to enforce the required text limits:

• title — maximum 68 characters;
• fact content — maximum 600 characters;
• optional image alt text — maximum 200 characters.

Summary of change

• Added and completed MediatR handlers for creating, updating, deleting, retrieving, and reordering facts.
• Added POST, PUT, DELETE, and reorder endpoints to FactController.
• Added validation for:
  • required title and fact content;
  • title length up to 68 characters;
  • fact content length up to 600 characters;
  • optional image alt text length up to 200 characters;
  • positive ImageId and StreetcodeId.
• Added checks that referenced Streetcode and Image entities exist.
• Renamed ImageDescription to ImageAlt because the value is stored on ImageDetails and belongs to the Image rather than to a specific Fact.
• Documented and implemented the image-scoped behavior: changing ImageAlt affects every entity that references the same Image.
• Defined update semantics for ImageAlt:
  • an omitted or null value preserves the existing Alt;
  • an empty or whitespace-only value clears the existing Alt;
  • a non-empty value is trimmed and saved.
• Added support for creating and updating ImageDetails when ImageAlt is supplied.
• Preserved the existing failure response when no facts exist for the requested Streetcode.
• Preserved and exposed DisplayOrder for each fact.
• New facts receive the next available display order.
• Facts are returned sorted by DisplayOrder.
• Added reordering support using an ordered list of fact IDs.
• Added validation against duplicate, missing, or foreign fact IDs during reorder.
• Updated delete behavior so the remaining facts receive continuous display positions.
• Added the LimitFactTitleLength EF Core migration.
• Added a migration guard that prevents reducing the title column length when existing values exceed 68 characters.
• Added unit tests for Fact handlers, DTO validation, image Alt semantics, ordering, and failure scenarios.

Testing approach

• Built the solution successfully with 0 errors.
• Ran all Fact unit tests successfully: 30 passed, 0 failed.
• Applied the EF Core migrations to a local SQL Server instance.
• Ran dotnet ef migrations has-pending-model-changes and confirmed that no model changes are pending.
• Verified DTO validation for:
  • title length up to 68 characters;
  • fact content length up to 600 characters;
  • image Alt length up to 200 characters;
  • required fields;
  • positive ImageId and StreetcodeId.
• Verified through Swagger:
  • successful Fact creation;
  • retrieval and sorting by DisplayOrder;
  • successful Fact update;
  • successful Fact deletion;
  • automatic order normalization after deletion;
  • successful reorder and persistence of the new order.
• Added tests confirming that:
  • omitting ImageAlt preserves the existing image Alt;
  • an empty or whitespace-only ImageAlt clears it;
  • a Streetcode without facts produces the existing failure response.

Scope note

ImageAlt is stored on ImageDetails and is therefore image-scoped. If several facts, arts, or Streetcodes reference the same Image, changing its Alt through one Fact changes it for every other reference to that Image.

Authentication and authorization are not implemented in this PR because the project does not currently register an authentication scheme. Authorization of admin endpoints is handled by separate authentication and access-control tasks.

The modal window, dynamic character counters, action icons, and drag-and-drop interface are frontend responsibilities. This PR provides the backend CRUD and reorder operations required by those features.

Known limitation

DisplayOrder for a newly created Fact is calculated as the current maximum order plus one. There is currently no unique constraint on (StreetcodeId, DisplayOrder), so two concurrent create requests for the same Streetcode may receive the same position. Database-level enforcement and conflict retry should be handled in a separate follow-up task.

Follow-up

Image extension validation for .jpeg, .jpg, .png, and .webp belongs to the shared ImageController/Create upload flow and will be handled in a separate task.

Closes #50

CHECK LIST

• CI passed
• Code coverage >=95%
• PR is reviewed manually again (to make sure the code is ready)
• All reviewers agreed to merge the PR
• I've checked the new feature as a logged-in and logged-out user if needed
• PR meets all conventions

[DrFaust555]

1. ImageDescription is stored on ImageDetails, which belongs to the Image, not to the Fact. An Image can be referenced by other facts, arts and streetcodes. Consequences: two facts sharing one image cannot have different descriptions, and writing a description through a fact rewrites the alt text everywhere that image is used. Either put the description on the Fact, or state in the description that it is image scoped and rename the field to ImageAlt so callers are not misled.

2. UpdateFactHandler sets image.ImageDetails.Alt = null whenever ImageDescription is empty or absent from the request body. Combined with item 1, a fact update that simply omits the field wipes the alt text that another entity depends on. Decide the semantics: either only write Alt when the caller sent a value, or document that omitting it clears it.

3. GetFactByStreetcodeIdHandler no longer returns a failure when a streetcode has no facts. It now returns 200 with an empty array. The old behaviour was 400 with "Cannot find any fact by the streetcode id". This is a breaking change for existing clients and it is not in the description. Add it, or keep the failure.

Also:

4. CreateFactHandler computes DisplayOrder as Max(DisplayOrder) + 1 with no unique constraint on (StreetcodeId, DisplayOrder). Two concurrent creates produce two facts at the same position. Either add the constraint and retry on conflict, or note the limitation.

On your open question: image extension validation belongs to the image upload endpoint, not here. Raise it as a separate task against ImageController/Create.

[DrFaust555]

The description says "Preserved the existing failure response when no facts
exist". That is not accurate. On dev the if (fact is null) branch in
GetFactByStreetcodeIdHandler is unreachable: RepositoryBase.GetAllAsync
returns ToListAsync(), which is an empty list, never null. dev therefore
answers 200 with an empty array. The PR changes that to 400. Describe it as a
behaviour change and confirm it with the frontend.

2. Image extension validation is only mentioned in the description as a
   follow-up. Open the actual issue against ImageController/Create so it is
   tracked.

3. ReorderFactsHandler: when a streetcode has no facts and OrderedFactIds is
   empty, SetEquals passes, UpdateRange gets an empty collection,
   SaveChangesAsync() returns 0 and the handler reports "Failed to reorder facts
   for streetcode with id: N". A no-op reorder should succeed.

4. FactUpdateCreateDto is validated with DataAnnotations, while PR #76 moves
   request validation into a FluentValidation MediatR pipeline. After both merge,
   fact create/update will return a different 400 payload than the rest of the
   API, and CreateFactCommand, UpdateFactCommand and ReorderFactsCommand will
   have no validators at all - ReorderFactsCommand does not even check
   StreetcodeId. Agree the approach with #76 before merge.

5. The guard in 20260818144116_LimitFactTitleLength uses LEN([Title]), which
   ignores trailing spaces. A title padded past 68 characters passes the guard
   and then fails inside ALTER COLUMN. Use DATALENGTH([Title]) / 2.

6. Fix the StyleCop violations Sonar reports in the new test files: SA1101,
   SA1200, SA1633, SA1309, SA1000

[Laminate32]

Everything looks fine for me. Code is very clean & understandable. It's an approve! Well done! :)

[Loki22978964]

Good, safe migration of LimitFactTitleLength (with a THROW exception when attempting to truncate longer data), logically consistent Create/Delete/Reorder methods (correctly update DisplayOrder), strong test coverage (success/failure/edge cases), and removal of unnecessary nullable noise in DTOs/entities.

Fixes prior to the merge:

1. Logger bug in GetFactByStreetcodeIdHandler (critical)
   The constructor renamed the field to _loggerService, but inside the method body (if (fact is null)), the old name _logger—which no longer exists—is still being called. SonarCloud flagged this as a failure—it either fails to compile or leaves a dead field. The renaming needs to be completed.

2. Duplicate ImageAlt logic (important)
   The “trim + create/update ImageDetails” block is repeated almost identically in both CreateFactHandler and UpdateFactHandler. It should be moved to a shared method or service to avoid having to edit the logic in two places during future changes.

3. Sonar-style warnings (minor)
   A space before the closing parenthesis in records (CreateFactCommand, DeleteFactCommand, etc.), unchained Include+ThenInclude, and several methods in tests that can be made static. Easy to fix in a single pass.

4. Potential race condition when calculating DisplayOrder (worth considering)
   existingFacts.Max(f => f.DisplayOrder) + 1 without a unique constraint or locking—with parallel requests to create a fact for the same streetcode, a duplicate DisplayOrder is possible. This is unlikely for the admin panel, but it can be logged as tech debt or a unique index can be added.

5. Authorization of mutating endpoints (check)
   In FactController, attributes such as [Authorize] for Create/Update/Delete/Reorder are not visible. It’s worth checking whether this was overlooked, especially if the project has a convention to secure admin CRUD operations.

6. Validation of ImageAlt when null (check)
   Ensure that MustNotExceedLength(200, ...) for ImageAlt correctly allows null—so that a fact can be updated without changing the alt (the handler tests confirm this; the main thing is that the validator doesn’t break earlier in the pipeline).

[sonarqubecloud]

Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

[Loki22978964]

Everything looks fine for me

[Laminate32]

Nice job!