crud-credit-package-ui #149
Conversation
WalkthroughThis pull request introduces new functionalities across both backend and frontend code. In the backend, new imports and search methods are added to support pagination and credit package searches. In the frontend, role-based routing guards are enhanced, and new components, services, and internationalization resources are added to manage the creation, updating, and display of credit packages. The changes include modifications to controller endpoints, service interfaces/implementations, routing constants, Angular components, and translation files. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as CreditPackageController
participant Service as CreditPackageServiceImpl
participant Repository as CreditPackageRepository
Client->>+Controller: POST /search with SearchCriteriaDTO
Controller->>+Service: Call search(pageable)
Service->>+Repository: Execute paginated search
Repository-->>-Service: Return Page<CreditPackageEntity>
Service-->>-Controller: Return search results
Controller->>Client: ResponseEntity<SearchResultDTO<CreditPackageDTO>>
sequenceDiagram
participant Route
participant Guard as AdminGuard
participant AuthSvc as AuthService
participant Router
Route->>Guard: Evaluate access for SYSTEM_ADMIN
Guard->>AuthSvc: Check authentication & user role
AuthSvc-->>Guard: Return user role info
alt Authorized (SYSTEM_ADMIN)
Guard-->>Route: Return true (Access Granted)
else Unauthorized
Guard->>Router: Navigate to forbidden route
Guard-->>Route: Return false (Access Denied)
end
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (3)
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
huynhlephcvinh
requested review from
thongdanghoang,
GiaBaorr and
nganntqe170236
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (10)
sep490-frontend/src/app/app-routing.module.ts (1)
17-50: Consider extracting reusable role-check logic.
TheauthGuardcode block is repeated inAdminGuardwith minor modifications for role-based checks. This duplication can be extracted into a helper function that accepts the required role, reducing repetition and enhancing maintainability.sep490-frontend/src/app/modules/admin/components/package-credit/package-credit.component.ts (1)
100-100: Correct the comment to reflect packages.
The inline comment references "the selected user's ID," but we’re retrieving the package’s ID.- const pkgId = this.selected[0].id; // Retrieve the selected user's ID. + const pkgId = this.selected[0].id; // Retrieve the selected package's ID.sep490-frontend/src/app/modules/admin/admin-routing.module.ts (1)
19-24: Remove unnecessary template literals.The template literals (
${...}) are not needed for static strings without interpolation.- path: `${AppRoutingConstants.PACKAGE_CREDIT_DETAILS_PATH}`, + path: AppRoutingConstants.PACKAGE_CREDIT_DETAILS_PATH, - path: `${AppRoutingConstants.PACKAGE_CREDIT_DETAILS_PATH}/:id`, + path: AppRoutingConstants.PACKAGE_CREDIT_DETAILS_PATH + '/:id',sep490-frontend/src/app/modules/admin/services/package-credit.service.ts (1)
11-11: Consider moving the endpoint to routing constants.The endpoint string could be moved to
AppRoutingConstantsto maintain consistency with other API endpoints.sep490-enterprise/src/main/java/enterprise/services/impl/CreditPackageServiceImpl.java (2)
3-14: Consider organizing imports by category.The imports could be better organized into categories (e.g., core Java, Spring, domain, etc.) for better readability.
// Core Java imports import java.util.*; // Spring Framework imports import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; import org.springframework.stereotype.Service; import org.springframework.util.CollectionUtils; // Domain imports import enterprise.entities.CreditPackageEntity; import enterprise.entities.PaymentEntity; import enterprise.repositories.CreditPackageRepository; import enterprise.services.CreditPackageService; // DTO imports import enterprise.dtos.PaymentCriteriaDTO; import green_buildings.commons.api.dto.SearchCriteriaDTO; // Utility imports import commons.springfw.impl.utils.SecurityUtils;
52-55: Consider enhancing the search method with filtering capabilities.The current implementation returns all records without any filtering. Consider adding support for search criteria to filter results based on price range, number of credits, etc.
@Override public Page<CreditPackageEntity> search(Pageable pageable) { - return creditPackageRepository.findAll(pageable); + // Example implementation with filtering + return creditPackageRepository.findByPriceBetweenAndNumberOfCreditsBetween( + searchCriteria.getMinPrice(), + searchCriteria.getMaxPrice(), + searchCriteria.getMinCredits(), + searchCriteria.getMaxCredits(), + pageable + ); }sep490-frontend/src/app/modules/admin/components/create-update-package-credit/create-update-package-credit.component.ts (1)
28-33: Consider enhancing form validation rules.While basic validation is in place, consider adding:
- Maximum value validators for both fields
- Pattern validator for price to ensure proper currency format
- Custom validator to ensure price is proportional to credits
numberOfCredits: new FormControl(null, [ Validators.required, Validators.min(1), + Validators.max(10000), + this.validateCreditsAndPrice ]), price: new FormControl(null, [ Validators.required, Validators.min(1), + Validators.max(1000000000), + Validators.pattern(/^\d+$/), + this.validateCreditsAndPrice ])sep490-frontend/src/app/modules/admin/components/package-credit/package-credit.component.html (1)
35-52: Enhance accessibility for action buttons.The action buttons lack proper ARIA labels and keyboard navigation support.
<p-button icon="pi pi-pencil" [rounded]="true" [text]="true" severity="info" + [attr.aria-label]="'Edit ' + rowData.numberOfCredits + ' credits package'" + [tabindex]="0" (onClick)="onEdit(rowData)" /> <p-button icon="pi pi-trash" [rounded]="true" [text]="true" severity="danger" + [attr.aria-label]="'Delete ' + rowData.numberOfCredits + ' credits package'" + [tabindex]="0" (onClick)="onDelete(rowData)" />sep490-frontend/src/app/modules/admin/components/create-update-package-credit/create-update-package-credit.component.html (1)
56-56: CSS Class Naming CheckThe class attribute on the button container includes
*:min-w-24. This notation is unusual for Tailwind CSS or standard CSS classes. Verify whether this is intentional or a typo that needs correction.sep490-frontend/src/assets/i18n/en.json (1)
212-235: New Translation Section for Package CreditThe new
"packageCredit"object under"admin"includes properties for the title, table headers, input prompts, and messages for success and error outcomes. One point to note: the error message "Package Credit deleted unsuccessfully" is somewhat awkward. Consider revising it to "Failed to delete Package Credit" for better clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
sep490-enterprise/src/main/java/enterprise/repositories/CreditPackageRepository.java(1 hunks)sep490-enterprise/src/main/java/enterprise/rest/CreditPackageController.java(2 hunks)sep490-enterprise/src/main/java/enterprise/services/CreditPackageService.java(2 hunks)sep490-enterprise/src/main/java/enterprise/services/impl/CreditPackageServiceImpl.java(2 hunks)sep490-frontend/src/app/app-routing.constant.ts(1 hunks)sep490-frontend/src/app/app-routing.module.ts(3 hunks)sep490-frontend/src/app/modules/admin/admin-routing.module.ts(2 hunks)sep490-frontend/src/app/modules/admin/admin.module.ts(1 hunks)sep490-frontend/src/app/modules/admin/components/create-update-package-credit/create-update-package-credit.component.html(1 hunks)sep490-frontend/src/app/modules/admin/components/create-update-package-credit/create-update-package-credit.component.ts(1 hunks)sep490-frontend/src/app/modules/admin/components/package-credit/package-credit.component.html(1 hunks)sep490-frontend/src/app/modules/admin/components/package-credit/package-credit.component.ts(1 hunks)sep490-frontend/src/app/modules/admin/services/package-credit.service.ts(1 hunks)sep490-frontend/src/assets/i18n/en.json(2 hunks)sep490-frontend/src/assets/i18n/vi.json(2 hunks)sep490-frontend/src/assets/i18n/zh.json(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- sep490-enterprise/src/main/java/enterprise/repositories/CreditPackageRepository.java
🧰 Additional context used
🪛 Biome (1.9.4)
sep490-frontend/src/app/modules/admin/components/create-update-package-credit/create-update-package-credit.component.ts
[error] 51-51: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build Java Modules (sep490-enterprise)
- GitHub Check: Build Java Modules (sep490-idp)
🔇 Additional comments (15)
sep490-frontend/src/app/app-routing.module.ts (3)
2-2: No issues with the updated imports and references.
Everything looks appropriate for the new authentication logic.Also applies to: 7-7, 15-15, 20-20
52-85: Verify handling of non-existent roles.
If a user doesn’t containUserRole.SYSTEM_ADMIN, the guard navigates toFORBIDDEN. Confirm that any upstream logic consistently assigns roles properly, so valid admins aren't inadvertently blocked, nor unprivileged users allowed.
110-110: Route-guard usage looks good.
Switching to theAdminGuardfor the admin route properly enforces system administrator access.sep490-frontend/src/app/modules/admin/components/package-credit/package-credit.component.ts (2)
128-157: Well-structured delete logic and user feedback.
Clearing theselectedarray after deletion and triggering a table refresh is a clean approach for UI consistency, with success/error messages providing immediate feedback.
71-88:✅ Verification successful
Verify defined translation keys.
The column headers use i18n keys like'admin.packageCredit.table.numberCredit'and'admin.packageCredit.table.price'. Ensure these keys exist in your translation files, so columns are properly labeled.
🏁 Script executed:
#!/bin/bash rg -A 2 "admin\.packageCredit\.table\.numberCredit" rg -A 2 "admin\.packageCredit\.table\.price"Length of output: 1846
Translation Keys Verified and Consistent
The verification confirms that both translation keys (
admin.packageCredit.table.numberCreditandadmin.packageCredit.table.price) are used consistently across components (in both the package-credit and create-update-package-credit components). No further changes are required.sep490-enterprise/src/main/java/enterprise/services/CreditPackageService.java (1)
3-7: New imports look consistent.
These additions align well with the introduction of paginated searches.sep490-frontend/src/app/modules/admin/admin.module.ts (1)
8-14: LGTM! Component properly integrated into the module.The
CreateUpdatePackageCreditComponentis correctly imported and declared in theAdminModule.sep490-frontend/src/app/app-routing.constant.ts (1)
26-26: LGTM! Routing constant properly defined.The constant follows the established naming pattern and is properly grouped under the Admin Module section.
sep490-frontend/src/app/modules/admin/components/create-update-package-credit/create-update-package-credit.component.html (2)
2-10: Conditional Rendering Syntax VerificationThe template uses
@if (isEdit)to conditionally render the header. In standard Angular templates the preferred approach is to use structural directives (e.g.*ngIf). Please confirm if you are using a preprocessor or a custom templating engine that supports the@ifsyntax.
58-72: Button Event Binding VerificationThe
<p-button>components use(onClick)="submit()"and(onClick)="reset()"for event handling. Typically, Angular components listen on the(click)event unless the button component explicitly definesonClickas an output. Please verify that the event bindings are correctly named for your custom component library.sep490-frontend/src/assets/i18n/zh.json (2)
6-6: Confirmation Message UpdateThe
defaultConfirmMessagehas been updated to "是否删除此行？" to reflect a row deletion context instead of user deletion. This update appears appropriate—please confirm that it aligns with the intended UX wording.
213-235: New Translation Section for Package CreditA new
"packageCredit"section under"admin"has been added with keys for title, table headers, input placeholders, and messages for success and error states. The messages currently indicate deletion outcomes (e.g. "信用包已删除"). Please confirm that these messages are meant for deletion actions and that their semantic context fits all intended credit package operations.sep490-frontend/src/assets/i18n/en.json (1)
6-6: Confirmation Message UpdateThe
defaultConfirmMessageis now "Do you want to delete this row?" which correctly reflects the new context for deletion confirmation.sep490-frontend/src/assets/i18n/vi.json (2)
4-4: Confirmation Message UpdateThe
defaultConfirmMessagehas been updated to "Bạn có muốn xóa hàng này không?" to match the new deletion context. This change looks good—please ensure it is consistent with other language translations.
212-236: New Translation Section for Package CreditA new
"packageCredit"block under"admin"has been added with keys for the title, table headers, input placeholders, and messages for success and error scenarios. The messages, which indicate deletion outcomes, should be reviewed to ensure they match the intended operations in your UI.
sep490-enterprise/src/main/java/enterprise/services/CreditPackageService.java
Show resolved
Hide resolved
sep490-frontend/src/app/modules/admin/services/package-credit.service.ts
Show resolved
Hide resolved
...ules/admin/components/create-update-package-credit/create-update-package-credit.component.ts
Show resolved
Hide resolved
sep490-enterprise/src/main/java/enterprise/rest/CreditPackageController.java
Show resolved
Hide resolved
sep490-frontend/src/app/modules/admin/components/package-credit/package-credit.component.html
Show resolved
Hide resolved
Summary by CodeRabbit
New Features
Refactor
Chores
These improvements empower administrators with robust tools for managing credit packages through an enriched UI and backend enhancements.