Conditional View Access For Discoverable Applications in MyAccount #7399
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #7399 +/- ##
=======================================
Coverage 41.85% 41.85%
=======================================
Files 42 42
Lines 939 939
Branches 217 230 +13
=======================================
Hits 393 393
+ Misses 546 502 -44
- Partials 0 44 +44
Flags with carried forward coverage won't be shown. Click here to find out more. |
| @@ -144,6 +144,10 @@ export const userstores: userstoresNS = { | |||
| } | |||
| }, | |||
| fetchUserstores: { | |||
| error: { | |||
| description: "{{description}}", | |||
There was a problem hiding this comment.
This placeholder was used to show the error description returned from backend, but we no longer show the errors returned from the backend and always show a end user friendly generic error message from the frontend.
Therefore, let's add such a description here.
@JayaShakthi97 will be able to provide context on this.
There was a problem hiding this comment.
IMO, there are two types of errors returned from the backend: client errors and server errors. I agree with you that server errors can encompass a wide range of issues (such as SQL-level problems). Therefore, it’s acceptable not to display detailed server errors and instead use a generic error message for all server errors. However, if the error response is a 400, I don’t think we need to handle it on the frontend by rephrasing the message. If the 400 error message isn’t clear, it should be fixed on the backend since all API users would face the same issue. Thus, it’s better to handle 400 errors and server errors differently. WDYT? @pavinduLakshan @JayaShakthi97 @NipuniBhagya @brionmario
| @@ -29,6 +29,10 @@ export interface ApplicationsResourceEndpointsInterface { | |||
| * Below route is to fetch the dcr configuration from server configurations api. | |||
| */ | |||
| dcrConfiguration: string; | |||
| /** | |||
| * Below route is to fetch the group data from the application api. | |||
There was a problem hiding this comment.
Shall we highlight the fact that group list is retrieved as metadata in the doc comment too?
eg:
| * Below route is to fetch the group data from the application api. | |
| * Below route is to fetch the group list as metadata from the application API. |
| * under the License. | ||
| */ | ||
|
|
||
| .application-general-discoverable-groups { |
There was a problem hiding this comment.
Let's add a parent class to application general sections and wrap discoverable-groups inside it, so that any other application general tab related styles can go under the parent class
eg:
.application-general-settings {
.discoverable-groups {
}
/* any other styles */
}
Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. |
Purpose
Related Issues
Related PRs
Checklist
Security checks
UI