Repository navigation
Commit 2afd18a
committed
chore: remove usage of getattr()
Remove usage of getattr(self, "_update_uses_post", False)
Instead add it to class and set default value to False.
Add a tests that shows it is set to True for the
ProjectMergeRequestApprovalManager and ProjectApprovalManager classes.1 parent 8ace76a commit 2afd18a
2 files changed
Lines changed: 27 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -331,6 +331,7 @@ class UpdateMixin(_RestManagerBase): | |||
| 331 | 331 | _parent = Optional[base.RESTObject] | |
| 332 | 332 | _parent_attrs = Dict[str, Any] | |
| 333 | 333 | _path: Optional[str] | |
| 334 | + _update_uses_post: bool = False | ||
| 334 | 335 | gitlab: gitlab.Gitlab | |
| 335 | 336 | ||
| 336 | 337 | def _check_missing_update_attrs(self, data: Dict[str, Any]) -> None: | |
@@ -357,7 +358,7 @@ def _get_update_method( | |||
| 357 | 358 | Returns: | |
| 358 | 359 | object: http_put (default) or http_post | |
| 359 | 360 | """ | |
| 360 | - if getattr(self, "_update_uses_post", False): | ||
| 361 | + if self._update_uses_post: | ||
| 361 | 362 | http_method = self.gitlab.http_post | |
| 362 | 363 | else: | |
| 363 | 364 | http_method = self.gitlab.http_put | |
Lines changed: 25 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -2,9 +2,12 @@ | |||
| 2 | 2 | Gitlab API: https://docs.gitlab.com/ee/api/merge_request_approvals.html | |
| 3 | 3 | """ | |
| 4 | 4 | ||
| 5 | + import copy | ||
| 6 | + | ||
| 5 | 7 | import pytest | |
| 6 | 8 | import responses | |
| 7 | - import copy | ||
| 9 | + | ||
| 10 | + import gitlab | ||
| 8 | 11 | ||
| 9 | 12 | ||
| 10 | 13 | approval_rule_id = 1 | |
@@ -230,6 +233,17 @@ def resp_snippet(): | |||
| 230 | 233 | yield rsps | |
| 231 | 234 | ||
| 232 | 235 | ||
| 236 | + def test_project_approval_manager_update_uses_post(project, resp_snippet): | ||
| 237 | + """Ensure the | ||
| 238 | + gitlab.v4.objects.merge_request_approvals.ProjectApprovalManager object has | ||
| 239 | + _update_uses_post set to True""" | ||
| 240 | + approvals = project.approvals | ||
| 241 | + assert isinstance( | ||
| 242 | + approvals, gitlab.v4.objects.merge_request_approvals.ProjectApprovalManager | ||
| 243 | + ) | ||
| 244 | + assert approvals._update_uses_post == True | ||
| 245 | + | ||
| 246 | + | ||
| 233 | 247 | def test_list_merge_request_approval_rules(project, resp_snippet): | |
| 234 | 248 | approval_rules = project.mergerequests.get(1).approval_rules.list() | |
| 235 | 249 | assert len(approval_rules) == 1 | |
@@ -239,6 +253,11 @@ def test_list_merge_request_approval_rules(project, resp_snippet): | |||
| 239 | 253 | ||
| 240 | 254 | def test_update_merge_request_approvals_set_approvers(project, resp_snippet): | |
| 241 | 255 | approvals = project.mergerequests.get(1).approvals | |
| 256 | + assert isinstance( | ||
| 257 | + approvals, | ||
| 258 | + gitlab.v4.objects.merge_request_approvals.ProjectMergeRequestApprovalManager, | ||
| 259 | + ) | ||
| 260 | + assert approvals._update_uses_post == True | ||
| 242 | 261 | response = approvals.set_approvers( | |
| 243 | 262 | updated_approval_rule_approvals_required, | |
| 244 | 263 | approver_ids=updated_approval_rule_user_ids, | |
@@ -254,6 +273,11 @@ def test_update_merge_request_approvals_set_approvers(project, resp_snippet): | |||
| 254 | 273 | ||
| 255 | 274 | def test_create_merge_request_approvals_set_approvers(project, resp_snippet): | |
| 256 | 275 | approvals = project.mergerequests.get(1).approvals | |
| 276 | + assert isinstance( | ||
| 277 | + approvals, | ||
| 278 | + gitlab.v4.objects.merge_request_approvals.ProjectMergeRequestApprovalManager, | ||
| 279 | + ) | ||
| 280 | + assert approvals._update_uses_post == True | ||
| 257 | 281 | response = approvals.set_approvers( | |
| 258 | 282 | new_approval_rule_approvals_required, | |
| 259 | 283 | approver_ids=new_approval_rule_user_ids, | |
0 commit comments