-
Notifications
You must be signed in to change notification settings - Fork 1
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
Admin UI: display error in quota dialog #514
Conversation
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.
The error message in your screenshot shows some random JS error. I don't know where exactly it comes from, but it's definitely not the one coming from the backbone (there is no undefined
in C#)
AdminUi/src/AdminUi/ClientApp/src/app/services/quotas-service/quotas.service.ts
Outdated
Show resolved
Hide resolved
...i/ClientApp/src/app/components/quotas/assign-quotas-dialog/assign-quotas-dialog.component.ts
Outdated
Show resolved
Hide resolved
...i/ClientApp/src/app/components/quotas/assign-quotas-dialog/assign-quotas-dialog.component.ts
Outdated
Show resolved
Hide resolved
@coderabbitai review |
Important Auto Review SkippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the To trigger a single review, invoke the WalkthroughThe updates focus on enhancing error handling and user feedback in the quota assignment dialog within an admin UI. They introduce a system to display error messages, refine quota creation logic with better conditions and error messages, and add a mechanism for refreshing data upon quota assignment. These changes aim to improve usability and ensure that quotas are only created with valid parameters. Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (8)
- AdminUi/src/AdminUi/ClientApp/src/app/components/quotas/assign-quotas-dialog/assign-quotas-dialog.component.css (1 hunks)
- AdminUi/src/AdminUi/ClientApp/src/app/components/quotas/assign-quotas-dialog/assign-quotas-dialog.component.html (1 hunks)
- AdminUi/src/AdminUi/ClientApp/src/app/components/quotas/assign-quotas-dialog/assign-quotas-dialog.component.ts (5 hunks)
- AdminUi/src/AdminUi/ClientApp/src/app/components/quotas/identity/identity-details/identity-details.component.ts (3 hunks)
- AdminUi/src/AdminUi/ClientApp/src/app/services/quotas-service/quotas.service.ts (2 hunks)
- Modules/Quotas/src/Quotas.Domain/Aggregates/Identities/Identity.cs (1 hunks)
- Modules/Quotas/src/Quotas.Domain/Aggregates/Tiers/Tier.cs (1 hunks)
- Modules/Quotas/src/Quotas.Domain/DomainErrors.cs (1 hunks)
Additional comments: 10
AdminUi/src/AdminUi/ClientApp/src/app/components/quotas/assign-quotas-dialog/assign-quotas-dialog.component.css (1)
- 12-23: The CSS for
.client-secret-warning-container
and.client-secret-warning
is well-defined, ensuring that error messages will be visually distinct and attention-grabbing for users. The choice of colors, padding, and font weight enhances readability and user experience.Modules/Quotas/src/Quotas.Domain/DomainErrors.cs (1)
- 14-14: The updated error message in
MaxValueCannotBeLowerOrEqualToZero
method now accurately reflects the validation logic, clarifying that the max value for a quota cannot be lower than zero. This change improves the clarity of error messages presented to the user or developer, aligning with the validation rules enforced in the domain logic.AdminUi/src/AdminUi/ClientApp/src/app/components/quotas/assign-quotas-dialog/assign-quotas-dialog.component.html (1)
- 34-36: The addition of a
div
element for displaying error messages whenerrorMessage
is not empty is a good practice for enhancing user feedback. This ensures that any issues encountered during the quota assignment process are communicated clearly to the user, improving the overall user experience.Modules/Quotas/src/Quotas.Domain/Aggregates/Tiers/Tier.cs (1)
- 23-23: The adjustment of the condition for
max
from<= 0
to< 0
in theCreateQuota
method is a logical correction that aligns with the updated validation rule. This ensures that quotas cannot be created with a max value of zero, which is consistent with the domain's validation requirements.AdminUi/src/AdminUi/ClientApp/src/app/services/quotas-service/quotas.service.ts (1)
- 13-22: The introduction of
refreshDataSubject
with a correspondingrefreshData$
observable and atriggerRefresh
method in theQuotasService
class is a well-implemented feature for enabling reactive data refreshes in the UI. This pattern allows components to react to changes and refresh their data accordingly, enhancing the responsiveness and interactivity of the application.AdminUi/src/AdminUi/ClientApp/src/app/components/quotas/assign-quotas-dialog/assign-quotas-dialog.component.ts (2)
- 24-25: The addition of
errorMessage
andisAllOk
properties to the component is a good practice for managing component state, especially in the context of asynchronous operations like quota assignment. These properties facilitate better error handling and success feedback within the UI.- 69-99: The enhancements to the
createIdentityQuota
method, including comprehensive error handling and success messaging, significantly improve the user experience by providing clear feedback on the outcome of quota assignments. The use oftriggerRefresh
to update the UI after a successful operation is a good practice, ensuring that the UI reflects the latest state.Modules/Quotas/src/Quotas.Domain/Aggregates/Identities/Identity.cs (1)
- 37-37: Adjusting the condition for
max
from<= 0
to< 0
in theCreateIndividualQuota
method correctly enforces the validation rule that the maximum value cannot be zero. This change ensures consistency in validation logic across different parts of the domain model, enhancing the robustness of the system.AdminUi/src/AdminUi/ClientApp/src/app/components/quotas/identity/identity-details/identity-details.component.ts (2)
- 82-84: Subscribing to
refreshData$
fromquotasService
to triggerloadIdentityAndTiers
upon data refresh is an effective way to ensure that the identity details component remains up-to-date with the latest data. This reactive approach enhances the user experience by automatically updating the UI in response to changes.- 189-192: The simplification of the
openAssignQuotaDialog
method, specifically the streamlined way of passingaddress
data toAssignQuotasDialogComponent
, improves the readability and maintainability of the code. This change ensures that only necessary data is passed to the dialog, reducing complexity and potential for errors.
… into assign-quota-dialog
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.
What's the state of this PR again? Is the error message problem fixed by now? If so, update the screenshot in the description please
...ClientApp/src/app/components/quotas/assign-quotas-dialog/assign-quotas-dialog.component.html
Outdated
Show resolved
Hide resolved
...i/ClientApp/src/app/components/quotas/assign-quotas-dialog/assign-quotas-dialog.component.ts
Outdated
Show resolved
Hide resolved
...i/ClientApp/src/app/components/quotas/assign-quotas-dialog/assign-quotas-dialog.component.ts
Outdated
Show resolved
Hide resolved
...i/ClientApp/src/app/components/quotas/assign-quotas-dialog/assign-quotas-dialog.component.ts
Show resolved
Hide resolved
.../ClientApp/src/app/components/quotas/identity/identity-details/identity-details.component.ts
Outdated
Show resolved
Hide resolved
AdminUi/src/AdminUi/ClientApp/src/app/components/quotas/tier/tier-edit/tier-edit.component.ts
Outdated
Show resolved
Hide resolved
… into assign-quota-dialog
Readiness checklist
Description
Summary by CodeRabbit