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

Added flag to prevent multiple triggers of getGroupingOwnersOnClick #998

Merged
merged 1 commit into from
Aug 21, 2024

Conversation

jarellb
Copy link
Contributor

@jarellb jarellb commented Aug 17, 2024

Ticket Link

GROUPINGS-1765

List of squashed commits

  1. Fixed codacy issue and shifted $scope.loadingOwner variables
  2. Corrected coding standard issue

Test Checklist

  • Exhibits Clear Code Structure:
  • Project Unit Tests Passed:
  • Project Jasmine Tests Passed:
  • Executes Expected Functionality:
  • Tested For Incorrect/Unexpected Inputs:

@jarellb jarellb requested a review from JorWo August 17, 2024 22:25
@JorWo
Copy link
Contributor

JorWo commented Aug 19, 2024

Just a very minor nitpick, I would want the $scope.loadingOwner variable inside $scope.handleGroupingOwnersOnError and $scope.handleGroupingOwnersOnSuccess to be placed right under $scope.loading.

There is also a Codacy issue that you can find in the list of checks at the bottom of this PR. I personally like that coding style you used for early returns in a function but we will need to follow the code style for this project in particular.

@jarellb jarellb force-pushed the dev-jarellab-1765 branch from 762e32a to 471a10b Compare August 19, 2024 21:37
@@ -469,6 +472,8 @@
* @param groupingPath - The path of the grouping to display owners
*/
$scope.getGroupingOwnersOnClick = (groupingPath) => {
if ($scope.loadingOwners === true) {return;} // Prevent multiple triggers while loading modal
Copy link
Contributor

@JorWo JorWo Aug 20, 2024

Choose a reason for hiding this comment

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

Sorry, one more thing if you could expand the brackets like this:

// Prevent multiple triggers while loading modal
if ($scope.loadingOwners === true) {
    return;
}

Turns out Codacy gave a not so good suggestion lol

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay got it, thank you

@jarellb jarellb force-pushed the dev-jarellab-1765 branch from 471a10b to bc74d56 Compare August 20, 2024 21:45
Copy link
Contributor

@JorWo JorWo left a comment

Choose a reason for hiding this comment

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

Great work!

@JorWo JorWo merged commit 2e4742f into uhawaii-system-its-ti-iam:main Aug 21, 2024
14 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