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

Fixed put.group_hunt_group method #80

Merged
merged 4 commits into from
Aug 12, 2024

Conversation

LivCurtis
Copy link
Contributor

Assigned an empty dict to "serviceInstanceProfile" as this needs to exist for the method to work. Also corrected a couple of things in the docstring.

Assigned an empty dict to "serviceInstanceProfile" as this needs to exist for the method to work.
Also corrected a couple of things in the docstring.
@@ -897,10 +897,10 @@ def group_hunt_groups_status(self, hunt_group_user_ids: list, status: bool =True


def group_hunt_group(self, service_provider_id: str, group_id: str, hunt_group_user_id: str, updates: dict):
Copy link
Owner

Choose a reason for hiding this comment

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

Change hunt_group_user_id to service_user_id in these args.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done :)

Copy link
Owner

@Jordan-Prescott Jordan-Prescott left a comment

Choose a reason for hiding this comment

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

Adjust for comments please.

@@ -913,7 +913,8 @@ def group_hunt_group(self, service_provider_id: str, group_id: str, hunt_group_u

updates["serviceProviderId"] = service_provider_id
updates["groupId"] = group_id
updates["serviceUserId"] = hunt_group_user_id
updates["serviceUserId"] = hunt_group_user_id
Copy link
Owner

Choose a reason for hiding this comment

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

Should match new argument

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done :)

@@ -913,7 +913,8 @@ def group_hunt_group(self, service_provider_id: str, group_id: str, hunt_group_u

updates["serviceProviderId"] = service_provider_id
updates["groupId"] = group_id
updates["serviceUserId"] = hunt_group_user_id
updates["serviceUserId"] = hunt_group_user_id
updates["serviceInstanceProfile"] = {}
Copy link
Owner

Choose a reason for hiding this comment

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

Check if this is in the updates otherwise here you will override it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done :)

Changed name of argument from 'hunt_group_user_id' to be 'service_user_id'
Updated group_hunt_group method to add a check if "serviceInstanceProfile" is given in the updates dict before overwriting it with an empty dict
Copy link
Contributor Author

@LivCurtis LivCurtis left a comment

Choose a reason for hiding this comment

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

Have made the suggested changes.

@@ -897,10 +897,10 @@ def group_hunt_groups_status(self, hunt_group_user_ids: list, status: bool =True


def group_hunt_group(self, service_provider_id: str, group_id: str, hunt_group_user_id: str, updates: dict):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done :)

@@ -913,7 +913,8 @@ def group_hunt_group(self, service_provider_id: str, group_id: str, hunt_group_u

updates["serviceProviderId"] = service_provider_id
updates["groupId"] = group_id
updates["serviceUserId"] = hunt_group_user_id
updates["serviceUserId"] = hunt_group_user_id
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done :)

@@ -913,7 +913,8 @@ def group_hunt_group(self, service_provider_id: str, group_id: str, hunt_group_u

updates["serviceProviderId"] = service_provider_id
updates["groupId"] = group_id
updates["serviceUserId"] = hunt_group_user_id
updates["serviceUserId"] = hunt_group_user_id
updates["serviceInstanceProfile"] = {}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done :)

Updated if statement to be 'if not...' rather than 'if ... is None'
@Jordan-Prescott Jordan-Prescott self-requested a review August 2, 2024 10:31
@LivCurtis LivCurtis linked an issue Aug 7, 2024 that may be closed by this pull request
Copy link
Owner

@Jordan-Prescott Jordan-Prescott left a comment

Choose a reason for hiding this comment

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

All looks good, nice work Liv!

@@ -913,19 +913,21 @@ def group_hunt_group(self, service_provider_id: str, group_id: str, hunt_group_u

updates["serviceProviderId"] = service_provider_id
updates["groupId"] = group_id
updates["serviceUserId"] = hunt_group_user_id
updates["serviceUserId"] = service_user_id
if updates.get("serviceInstanceProfile") is None:
Copy link
Owner

Choose a reason for hiding this comment

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

Here you can remove the 'is None' and add 'not'.

Explanation: In Python the If statement can be shortened to look at Boolean states, which it classes None as False. Therefore you can write and if statement like 'if not updates.get("serviceInstanceProfile"):' and this will trigger when that get method evaluates to a Boolean false.

@Jordan-Prescott Jordan-Prescott merged commit 34a7287 into main Aug 12, 2024
@Jordan-Prescott Jordan-Prescott deleted the liv_debug_put_group_hunt_group branch August 12, 2024 09:26
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.

PUT Hunt Group is failing to update
2 participants