Open
Conversation
jmendeza
force-pushed
the
feature/6061-lang
branch
from
ef00753 to
977705d
Compare
4 tasks
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 14 minutes and 23 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
WalkthroughAdds a new GET /api/2/system/available_languages endpoint, introduces a Language record, extends configuration service and implementations to expose available languages, updates SystemController and routing/result constants, and adds studio configuration entries plus security URL updates for the new endpoint. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant SystemController
participant ConfigurationService
participant ConfigurationServiceInternal
participant ConfigurationCache
participant StudioConfiguration
Client->>SystemController: GET /api/2/system/available_languages
SystemController->>ConfigurationService: getAvailableLanguages()
ConfigurationService->>ConfigurationServiceInternal: getAvailableLanguages()
ConfigurationServiceInternal->>ConfigurationCache: get(CONFIGURATION_AVAILABLE_LANGUAGES, loader)
alt cache hit
ConfigurationCache-->>ConfigurationServiceInternal: Collection<Language>
else cache miss
ConfigurationServiceInternal->>StudioConfiguration: getSubConfigs(CONFIGURATION_AVAILABLE_LANGUAGES)
StudioConfiguration-->>ConfigurationServiceInternal: List<config nodes>
ConfigurationServiceInternal->>ConfigurationServiceInternal: loadAvailableLanguages() (extract id,label)
ConfigurationServiceInternal->>ConfigurationCache: store Collection<Language>
ConfigurationCache-->>ConfigurationServiceInternal: Collection<Language>
end
ConfigurationServiceInternal-->>ConfigurationService: Collection<Language>
ConfigurationService-->>SystemController: Collection<Language>
SystemController-->>Client: 200 OK { response: {...}, languages: [...] }
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/main/java/org/craftercms/studio/api/v2/service/config/ConfigurationService.java (1)
267-272: Consider documenting the checked exception in the new API contract.For consistency with the rest of this interface, add an
@throws ServiceLayerExceptionline in this method’s Javadoc.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/craftercms/studio/api/v2/service/config/ConfigurationService.java` around lines 267 - 272, Add an `@throws` ServiceLayerException Javadoc tag to the getAvailableLanguages() method in the ConfigurationService interface so the checked exception is documented in the API contract; update the Javadoc block above the method declaration (the comment for getAvailableLanguages) to include a line like "@throws ServiceLayerException if an error occurs in the service layer" so the interface consistently documents checked exceptions.src/main/resources/crafter/studio/studio-config.yaml (1)
364-365: Remove duplicated forgot-password public URL pattern.
/api/2/users/forgot_password.*appears twice across Line 364 and Line 365. Keeping one improves maintainability with no behavior change.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/crafter/studio/studio-config.yaml` around lines 364 - 365, The public URL patterns list contains a duplicate entry of the same forgot-password regex '/api/2/users/forgot_password.*'; remove the duplicated occurrence so the list only contains a single '/api/2/users/forgot_password.*' entry (leave the other patterns like '/api/2/monitoring/.+' and '/api/2/users/set_password.*' unchanged) to improve maintainability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/api/studio-api.yaml`:
- Around line 8197-8224: The operation getAvailableLanguages in the OpenAPI
contract is advertised as "No permission required" but still inherits file-level
auth; update the operation (operationId: getAvailableLanguages) to include
security: [] to mark it as public and remove the '401' and '403' response
entries from this operation's responses so the spec matches the documented
public behavior; keep the '200' (and other non-auth-related responses like '500'
if applicable) intact.
In
`@src/main/java/org/craftercms/studio/impl/v2/service/configuration/internal/ConfigurationServiceInternalImpl.java`:
- Around line 731-753: getAvailableLanguages currently returns the cached
collection directly and loadAvailableLanguages builds a mutable ArrayList,
exposing shared mutable state via configurationCache and
CONFIGURATION_AVAILABLE_LANGUAGES; change loadAvailableLanguages to return an
unmodifiable collection (e.g., wrap the final List with
Collections.unmodifiableList) and ensure getAvailableLanguages does not return
the cached mutable instance directly (either cache the unmodifiable collection
or return a defensive copy) so callers cannot mutate the cached languages;
reference the methods getAvailableLanguages, loadAvailableLanguages and the
configurationCache/CONFIGURATION_AVAILABLE_LANGUAGES keys when making the
change.
In `@src/main/resources/crafter/studio/studio-config.yaml`:
- Line 363: The public-URL regex entry for the available languages endpoint is
an exact path and will not match query-string variants; update the string
"/api/2/system/available_languages" in the studio-config.yaml public URL list to
a regex that allows query strings (e.g., append ".*") so it reads
"/api/2/system/available_languages.*" (preserve the trailing comma and
surrounding list format) to prevent accidental auth requirements for requests
like /api/2/system/available_languages?...
---
Nitpick comments:
In
`@src/main/java/org/craftercms/studio/api/v2/service/config/ConfigurationService.java`:
- Around line 267-272: Add an `@throws` ServiceLayerException Javadoc tag to the
getAvailableLanguages() method in the ConfigurationService interface so the
checked exception is documented in the API contract; update the Javadoc block
above the method declaration (the comment for getAvailableLanguages) to include
a line like "@throws ServiceLayerException if an error occurs in the service
layer" so the interface consistently documents checked exceptions.
In `@src/main/resources/crafter/studio/studio-config.yaml`:
- Around line 364-365: The public URL patterns list contains a duplicate entry
of the same forgot-password regex '/api/2/users/forgot_password.*'; remove the
duplicated occurrence so the list only contains a single
'/api/2/users/forgot_password.*' entry (leave the other patterns like
'/api/2/monitoring/.+' and '/api/2/users/set_password.*' unchanged) to improve
maintainability.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b56857f2-5087-426f-bae7-663044826830
📒 Files selected for processing (10)
src/main/api/studio-api.yamlsrc/main/java/org/craftercms/studio/api/v2/service/config/ConfigurationService.javasrc/main/java/org/craftercms/studio/api/v2/utils/StudioConfiguration.javasrc/main/java/org/craftercms/studio/controller/rest/v2/RequestMappingConstants.javasrc/main/java/org/craftercms/studio/controller/rest/v2/ResultConstants.javasrc/main/java/org/craftercms/studio/controller/rest/v2/SystemController.javasrc/main/java/org/craftercms/studio/impl/v2/service/configuration/ConfigurationServiceImpl.javasrc/main/java/org/craftercms/studio/impl/v2/service/configuration/internal/ConfigurationServiceInternalImpl.javasrc/main/java/org/craftercms/studio/model/i18n/Language.javasrc/main/resources/crafter/studio/studio-config.yaml
...aftercms/studio/impl/v2/service/configuration/internal/ConfigurationServiceInternalImpl.java
Show resolved
Hide resolved
jmendeza
force-pushed
the
feature/6061-lang
branch
3 times, most recently
from
ce65196 to
108eec6
Compare
jmendeza
force-pushed
the
feature/6061-lang
branch
from
108eec6 to
c2c8f20
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Available languages API v2
craftercms/craftercms#6061
Summary by CodeRabbit
New Features
Chores