Skip to content

Conversation

@AndyButland
Copy link
Contributor

Prerequisites

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

Resolves: #20639

Description

This PR fixes where we can - and comments where we can't - controller permission integration tests that were asserting on an error response.

There is an actual error revealed by one of these and noted in the linked issue, which is fixed and specific integration tests around this component - PropertyTypeUsageService - have been added.

Testing

Verify integration tests pass.

…rollers (closes #20602) (#20608)

* Restore backward compatibility for file system based tree controllers.

* Aligned obsoletion messages.
Copilot AI review requested due to automatic review settings October 24, 2025 12:23
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

This PR addresses flaky integration tests that were incorrectly asserting on error responses instead of success responses. The key change involves fixing the PropertyTypeUsageService implementation that was causing actual errors, and updating test assertions to expect HttpStatusCode.OK instead of InternalServerError where appropriate.

Key changes:

  • Fixed PropertyTypeUsageRepository to use WhereIn instead of Contains for database queries, resolving the underlying bug
  • Added new integration tests for PropertyTypeUsageService to verify the fix
  • Updated multiple controller permission tests to assert on HttpStatusCode.OK instead of error responses

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
PropertyTypeUsageServiceTests.cs New integration tests verifying the service correctly handles property value checks and content type validation
UmbracoIntegrationTestWithContent.cs Extracted content type key constant for reuse in tests
PropertyTypeUsageRepository.cs Fixed database query to use WhereIn instead of Contains, added XML documentation, and improved code formatting
PropertyTypeUsageService.cs Removed unused _contentTypeService field and marked parameter for future removal, added XML documentation
IPropertyTypeUsageService.cs Added XML documentation for interface
IPropertyTypeUsageRepository.cs Added XML documentation for repository interface methods
InviteUserControllerTests.cs Added clarifying comment explaining why InternalServerError is expected
IsUsedPropertyTypeControllerTests.cs Changed assertion from InternalServerError to OK after underlying bug fix
ValidateLogFileSizeLogViewerControllerTests.cs Changed assertion from InternalServerError to OK
LogLevelCountLogViewerControllerTests.cs Changed assertion from InternalServerError to OK
AllMessageTemplateLogViewerControllerTests.cs Changed assertion from InternalServerError to OK
AllLogViewerControllerTests.cs Changed assertion from InternalServerError to OK
ExecuteActionHealthCheckControllerTests.cs Changed assertion from InternalServerError to OK and added required Alias property to request

@AndyButland AndyButland force-pushed the v17/hotfix/fix-integration-tests branch from 0c838b9 to c3cead9 Compare October 24, 2025 12:23
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.

1 participant