change: set capability favorites to false, for unadjusted clients#2731
change: set capability favorites to false, for unadjusted clients#2731AlexAndBear wants to merge 2 commits into
Conversation
|
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 13 |
| Duplication | 5 |
AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
Pull Request Overview
The PR implements a global hardcoded change to the 'favorites' capability, setting it to false. While the title suggests this is for 'unadjusted clients', the implementation affects all clients, which constitutes a potential breaking change and a major logic contradiction.
Although Codacy marks the PR as 'Up to Standards', the modified file services/frontend/pkg/revaconfig/config.go is flagged as an uncovered complex file with a significant complexity increase (+13). The lack of unit tests and an empty PR description are major concerns that prevent a safe merge. This configuration change should be made configurable and verified with tests before proceeding.
About this PR
- The PR description is almost entirely empty (Motivation and Context, testing details, and checklists are incomplete). Furthermore, no unit tests were added to ensure this configuration change propagates correctly.
1 comment outside of the diff
services/frontend/pkg/revaconfig/config.go
line 390-446🟡 MEDIUM RISK
This 56-line block of configuration logic is identical to logic inservices/sharing/pkg/revaconfig/config.go. Large duplications like this should be refactored into a shared utility to reduce maintenance burden and prevent configuration drift.
Test suggestions
- Verify that the generated frontend configuration explicitly sets the favorites capability to false
- Generate unit tests for the
revaconfigpackage to cover the configuration mapping logic
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Verify that the generated frontend configuration explicitly sets the favorites capability to false
2. Generate unit tests for the `revaconfig` package to cover the configuration mapping logic
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
| "archivers": archivers, | ||
| "app_providers": appProviders, | ||
| "favorites": true, | ||
| "favorites": false, |
There was a problem hiding this comment.
🟡 MEDIUM RISK
The favorites capability is now hardcoded to false for all clients, contradicting the PR title's suggestion that this is targeted only at 'unadjusted' clients. If some clients still need this feature enabled, this global change will break it. Additionally, this file's complexity has increased (+13) without any test coverage, making this change fragile. Consider adding a Favorites boolean field to the Config struct to allow for better control and generate unit tests to cover the mapping logic.



Description
Related Issue
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: