-
Notifications
You must be signed in to change notification settings - Fork 17
feat/content-manager-api #63
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughAdds a Content Manager integration toggle and associated setting; calendar event fetching now conditionally queries the Content Manager collection-types endpoint with RBAC filtering when enabled, otherwise uses the public plugin endpoint. Includes translations and a settings default, plus a Strapi dependency version bump. Changes
Sequence Diagram(s)mermaid Note over UI,Settings: User opens calendar (reads Settings) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (1)
admin/src/components/Settings/GeneralSettings.tsx (1)
274-306: The new Content Manager toggle is well-implemented.The UI component follows the established pattern in this file, with proper internationalization and state management. The hint description clearly communicates the RBAC purpose.
🔎 Minor cleanup: Remove redundant empty Field.Hint
Line 304 renders an empty
<Field.Hint />component, which is redundant since the hint text is already provided via thehintprop onField.Root(line 277). Consider removing this line for cleaner code./> - <Field.Hint /> </Field.Root>
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
admin/src/components/Settings/GeneralSettings.tsxadmin/src/pages/CalendarPage.tsxadmin/src/translations/de.jsonadmin/src/translations/en.jsonadmin/src/translations/es.jsonadmin/src/utils/defaultSettings.tspackage.jsontypes.ts
🧰 Additional context used
🧬 Code graph analysis (2)
admin/src/pages/CalendarPage.tsx (1)
admin/src/pluginId.ts (1)
PLUGIN_ID(1-1)
admin/src/components/Settings/GeneralSettings.tsx (1)
admin/src/utils/getTranslation.ts (1)
getTranslation(5-5)
🔇 Additional comments (8)
types.ts (1)
9-9: LGTM!The addition of the
contentManagerboolean field toSettingsTypeis clean and consistent with the existing type structure.admin/src/utils/defaultSettings.ts (1)
10-10: LGTM!Setting
contentManagertotrueby default is a sensible choice, as it ensures that users see only the content they have permissions for, aligning with RBAC best practices.admin/src/translations/de.json (1)
37-40: Translation keys are consistently present across all language files.All three language files (en.json, es.json, de.json) contain the four required Content Manager plugin translation keys (label, description, off, on) with appropriate formatting and localized content for each language.
admin/src/translations/en.json (1)
37-40: LGTM! Translation keys are well-structured.The new translation keys follow the existing pattern and provide clear labels for the Content Manager plugin toggle feature.
admin/src/pages/CalendarPage.tsx (3)
24-24: LGTM! Proper use of useFetchClient.The
useFetchClienthook provides authenticated API calls, which is appropriate for fetching calendar events with RBAC enforcement.
227-227: LGTM! Correct usage of FullCalendar events prop.Switching from a URL string to a function reference allows for dynamic event fetching based on the
contentManagersetting, which is the intended behavior for this feature.
168-168: The status filter logic is correct.The concern is unfounded. When
draftsistrue, the status filter isundefined(showing all content including drafts), and whendraftsisfalse, the status filter is'published'(showing only published content). This aligns perfectly with the label "Display drafts" in the settings UI and the intended behavior of the toggle.admin/src/translations/es.json (1)
37-40: LGTM! Spanish translations are accurate and consistent.The Spanish translation keys are well-structured and align with the English translations, providing proper localization for the Content Manager plugin toggle feature.
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.