utils.can_put_annotation : 引数の追加
#728
Conversation
Titleadd test code Description
Changes walkthrough 📝
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
![kci-pr-agent[bot]](https://avatars.githubusercontent.com/in/1245598?s=60&v=4){kind=small}
| "histories_by_phase": [{"phase": "annotation", "phase_stage": 1, "worked": True, "account_id": self.OTHER_ACCOUNT_ID}], | ||
| "account_id": self.OTHER_ACCOUNT_ID, | ||
| } | ||
| actual = can_put_annotation(task, self.MY_ACCOUNT_ID, project_member_role=ProjectMemberRole.OWNER) |
There was a problem hiding this comment.
Suggestion: テスト名ではproject_member_role=Noneのケースを検証すると記載されていますが、実際にはOWNERを渡しているため、引数をNoneに修正してください。 [general, importance: 5]
| actual = can_put_annotation(task, self.MY_ACCOUNT_ID, project_member_role=ProjectMemberRole.OWNER) | |
| actual = can_put_annotation(task, self.MY_ACCOUNT_ID, project_member_role=None) |
tests/test_utils.py
Outdated
| actual = can_put_annotation(task, self.MY_ACCOUNT_ID, project_member_role=ProjectMemberRole.OWNER) | ||
| assert actual is False | ||
|
|
||
| def test_can_put_annotation_project_member_role_owner_with_history_different_account(self): |
There was a problem hiding this comment.
Suggestion: テスト名は“different_account”となっていますが、実際のシナリオは同じアカウントを想定しているため、テスト名を_same_accountに変更してください。 [general, importance: 4]
| def test_can_put_annotation_project_member_role_owner_with_history_different_account(self): | |
| def test_can_put_annotation_project_member_role_owner_with_history_same_account(self): |
There was a problem hiding this comment.
Pull Request Overview
This PR adds a project_member_role parameter to the can_put_annotation utility function to handle different behaviors based on project member roles. The function previously assumed the user was always a project owner, but now supports different logic for owners, accepters, and workers.
- Added
project_member_roleparameter with optional typing and default None behavior - Updated function logic to handle different project member roles (OWNER, ACCEPTER, WORKER)
- Added comprehensive test coverage for the new parameter and different role scenarios
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| annofabapi/utils.py | Modified can_put_annotation function to accept and handle project_member_role parameter with role-specific logic |
| tests/test_utils.py | Added test class TestCanPutAnnotation with comprehensive test cases for different project member roles |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| import dateutil.tz | ||
|
|
||
| from annofabapi.models import Task, TaskHistory, TaskHistoryShort, TaskPhase | ||
| from annofabapi.models import ProjectMemberRole, Task, TaskHistory, TaskHistoryShort, TaskPhase |
There was a problem hiding this comment.
Missing import for Optional type hint used in line 148. Add from typing import Optional to the imports.
| project_member_role: プロジェクトメンバーロール。Noneの場合、プロジェクトオーナであるとみなします。 | ||
| Returns: | ||
| Trueならば、タスクの状態を変更せずに`put_annotation` APIを実行できる。 | ||
| Trueならば、タスクの担当者を変更せずに`put_annotation` APIを実行できる。 | ||
| Falseならば、タスクの担当者を変更してから、`put_annotation` APIを実行する必要がある。 |
There was a problem hiding this comment.
The documentation states that None is treated as project owner, but this behavior change should be more explicitly documented since it affects the function's backward compatibility.
1 participant
ロールメンバーごとに挙動が異なるので、ロールメンバーを渡せるようにしました。