Skip to content
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

Improved: logic to show rejection reason options based on admin permission (#438) #440

Merged
merged 4 commits into from
Nov 13, 2024

Conversation

amansinghbais
Copy link
Contributor

Related Issues

#438

Short Description and Why It's Useful

Showing all rejection reasons only if user is admin else showing only the reasons from the bopis group.

Screenshots of Visual Changes before/after (If There Are Any)

Contribution and Currently Important Rules Acceptance

@ravilodhi
Copy link
Collaborator

ravilodhi commented Nov 6, 2024

OMS 5.18.0 Dependant. A view entity is added and the related PR got merged on Oct 17, 2024.

@@ -92,7 +92,8 @@ export default defineComponent({
modalController.dismiss({ dismissed: true });
},
getRejectReasonDescription(rejectReasonEnumId: string) {
return this.rejectReasons.find((reason: any) => reason.enumId === rejectReasonEnumId)?.description;
const reason = this.rejectReasons.find((reason: any) => reason.enumId === rejectReasonEnumId)
return reason?.description ? reason.description : reason?.enumDescription;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These checks might lead to an incorrect description. My understanding is that we’re fetching data from two different view entities:

For Admin users: EnumTypeChildAndEnum
For other users: EnumerationGroupAndMember

In the case of an Admin user, the current logic works as expected. However, for other users, it may not work correctly because the EnumerationGroupAndMember view contains both description and enumDescription fields. Here, description refers to EnumGroup.description, while enumDescription refers to Enumeration.description.

Let me know if I am thinking wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will not give issue, since we are not fetching description field in case of EnumerationGroupAndMember, so we won't have description and EnumDescription at the same time.

@ravilodhi ravilodhi merged commit 27fc3ee into hotwax:main Nov 13, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants