Conversation
📝 WalkthroughWalkthroughThis PR migrates contact form configuration from the global Lychee store to the ModulesRightsResource, restricting visibility of unread message counts to administrators. A visual badge component is integrated into the left menu to display message counters where available. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
resources/js/composables/contextMenus/leftMenu.ts (1)
12-26:⚠️ Potential issue | 🟡 MinorMissing
numproperty inMenyTypedefinition.The
MenyTypeunion type does not include anumproperty, but line 163 addsnum: initData.value.modules.messages_countto a menu item, andLeftMenu.vue(line 29) accessesitem.num > 0for badge rendering. This type mismatch will either cause TypeScript compilation errors or runtime issues.🐛 Proposed fix to add the optional num property
export type MenyType = | { label: string; icon: string; route?: string; url?: string; target?: string; access: boolean; seTag?: boolean; + num?: number; command?: () => void; } | { label: string; items: MenyType[]; };
🧹 Nitpick comments (5)
app/Http/Resources/Rights/ModulesRightsResource.php (3)
191-211: Method naming inconsistency: consider returningboolor renaming tosetContactEnabled.All other module check methods in this class (
isMapEnabled,isModFrameEnabled,isModFlowEnabled, etc.) returnbooland follow the pattern of returning a value assigned in the constructor. This method mutates properties directly and returnsvoid, which breaks the established pattern and may confuse future maintainers.Consider either:
- Renaming to
setContactEnabled()to indicate the side-effect nature, or- Refactoring to return
boollike the other methods and settingmessages_countseparately.
199-199: Inconsistent config access pattern.This line uses
resolve(ConfigManager::class)->getValueAsBool(...)while the rest of the class usesrequest()->configs()->getValueAsBool(...). Although functionally equivalent (per the macro defined inAppServiceProvider), maintaining consistency improves readability.♻️ Suggested change for consistency
- if (!resolve(ConfigManager::class)->getValueAsBool('contact_form_enabled')) { + if (!request()->configs()->getValueAsBool('contact_form_enabled')) {With this change, you can also remove the
use App\Repositories\ConfigManager;import on line 17.
210-210: UnnecessarytoBase()call beforecount().The
->toBase()conversion is typically used to avoid Eloquent model hydration, butcount()doesn't hydrate models regardless. This call has no performance benefit here and adds noise.♻️ Suggested simplification
- $this->messages_count = ContactMessage::query()->where('is_read', '=', false)->toBase()->count(); + $this->messages_count = ContactMessage::query()->where('is_read', '=', false)->count();resources/js/menus/LeftMenu.vue (2)
29-32: Leading space in pt:badge:class attribute.The value
' outline-0'has a leading space which is likely unintentional. This won't break functionality but is a minor quality issue.♻️ Remove leading space
- <OverlayBadge v-if="item.num > 0" severity="primary" :pt:badge:class="' outline-0'"> + <OverlayBadge v-if="item.num > 0" severity="primary" :pt:badge:class="'outline-0'">
185-189: Unscoped CSS affects global styles.This
<style lang="css">block is not scoped, so.p-overlaybadge .p-badgestyles will apply globally to all OverlayBadge components in the application. If this is intentional (to establish a consistent badge style), consider adding a comment to clarify. Otherwise, use<style scoped>or a more specific selector.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 862280e0-e3d6-4d68-b7c7-c62555cc1b9f
📒 Files selected for processing (6)
app/Http/Resources/GalleryConfigs/InitConfig.phpapp/Http/Resources/Rights/ModulesRightsResource.phpresources/js/composables/contextMenus/leftMenu.tsresources/js/lychee.d.tsresources/js/menus/LeftMenu.vueresources/js/stores/LycheeState.ts
💤 Files with no reviewable changes (2)
- resources/js/stores/LycheeState.ts
- app/Http/Resources/GalleryConfigs/InitConfig.php
Codecov Report❌ Patch coverage is 🚀 New features to boost your workflow:
|
1 participant
Summary by CodeRabbit
Release Notes
New Features
Refactor