[WEB-5537]refactor: rename IssueUserProperty to ProjectUserProperty and update related references #8206
Conversation
…related references across the codebase
pablohashescobar
requested review from
Copilot and
dheeru0198
and removed request for
Copilot
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughIssueUserProperty was renamed and moved to ProjectUserProperty with added fields (preferences, sort_order) and constraints; code references updated across serializers, views, URLs, tasks, management commands, and tests; two migrations (schema + data) perform the rename and data migration; APIToken gained allowed_rate_limit. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/api/plane/db/models/api.py (1)
35-39: Newallowed_rate_limitfield is consistent with migration; consider minor cleanupsThe
allowed_rate_limitCharField with default"60/min"looks fine and matches the migration; this should be safe for existing tokens. You may also want to (optionally) addhelp_textclarifying the expected format and fix the existing typo"API Tokems"inverbose_name_pluralwhile you’re in this model.apps/api/plane/db/migrations/0113_alter_issueuserproperty_table.py (1)
1-38: Migration sequence and new fields look consistent with the model changesThe migration cleanly renames the
IssueUserPropertytable/model toProjectUserPropertyand adds theallowed_rate_limit,preferences, andsort_orderfields in line with the model definitions. If you want extra long‑term safety, you could inline a simple dict default forpreferencesin this migration instead of importingget_default_preferences, but it’s not strictly required.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
apps/api/plane/api/views/project.py(3 hunks)apps/api/plane/app/serializers/issue.py(2 hunks)apps/api/plane/app/urls/issue.py(1 hunks)apps/api/plane/app/views/issue/base.py(3 hunks)apps/api/plane/app/views/project/base.py(3 hunks)apps/api/plane/app/views/project/invite.py(2 hunks)apps/api/plane/app/views/project/member.py(3 hunks)apps/api/plane/bgtasks/workspace_seed_task.py(2 hunks)apps/api/plane/db/management/commands/create_project_member.py(2 hunks)apps/api/plane/db/migrations/0113_alter_issueuserproperty_table.py(1 hunks)apps/api/plane/db/migrations/0114_auto_20251201_0857.py(1 hunks)apps/api/plane/db/models/__init__.py(1 hunks)apps/api/plane/db/models/api.py(1 hunks)apps/api/plane/db/models/issue.py(0 hunks)apps/api/plane/db/models/project.py(1 hunks)apps/api/plane/tests/contract/app/test_project_app.py(3 hunks)
💤 Files with no reviewable changes (1)
- apps/api/plane/db/models/issue.py
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-07-23T18:18:06.875Z
Learnt from: NarayanBavisetti
Repo: makeplane/plane PR: 7460
File: apps/api/plane/app/serializers/draft.py:112-122
Timestamp: 2025-07-23T18:18:06.875Z
Learning: In the Plane codebase serializers, workspace_id is not consistently passed in serializer context, so parent issue validation in DraftIssueCreateSerializer only checks project_id rather than both workspace_id and project_id. The existing project member authentication system already validates that users can only access projects they belong to, providing sufficient security without risking breaking functionality by adding workspace_id validation where the context might not be available.
Applied to files:
apps/api/plane/app/views/project/member.pyapps/api/plane/app/serializers/issue.pyapps/api/plane/api/views/project.pyapps/api/plane/tests/contract/app/test_project_app.pyapps/api/plane/bgtasks/workspace_seed_task.py
📚 Learning: 2025-10-29T09:17:54.815Z
Learnt from: NarayanBavisetti
Repo: makeplane/plane PR: 7905
File: apps/api/plane/app/views/search/base.py:241-276
Timestamp: 2025-10-29T09:17:54.815Z
Learning: In apps/api/plane/app/views/search/base.py, the `filter_intakes` method uses `Issue.objects` (base manager) instead of `Issue.issue_objects` (custom manager) because the custom manager filters out all intake statuses, which would prevent querying pending and snoozed intake issues.
Applied to files:
apps/api/plane/api/views/project.pyapps/api/plane/app/views/issue/base.py
📚 Learning: 2025-09-12T07:29:36.083Z
Learnt from: dheeru0198
Repo: makeplane/plane PR: 7625
File: apps/api/plane/bgtasks/workspace_seed_task.py:95-98
Timestamp: 2025-09-12T07:29:36.083Z
Learning: In the Plane codebase, workspace seed data should have project features (cycle_view, module_view, issue_views_view) enabled by default, even when regular project creation has these features disabled. This provides users with a complete demo experience in the seeded workspace.
Applied to files:
apps/api/plane/bgtasks/workspace_seed_task.py
🧬 Code graph analysis (14)
apps/api/plane/app/views/project/member.py (1)
apps/api/plane/db/models/project.py (2)
Project(64-158)ProjectUserProperty(317-348)
apps/api/plane/app/serializers/issue.py (1)
apps/api/plane/db/models/project.py (1)
ProjectUserProperty(317-348)
apps/api/plane/db/models/__init__.py (1)
apps/api/plane/db/models/project.py (1)
ProjectUserProperty(317-348)
apps/api/plane/db/management/commands/create_project_member.py (1)
apps/api/plane/db/models/project.py (1)
ProjectUserProperty(317-348)
apps/api/plane/app/urls/issue.py (1)
apps/api/plane/app/views/issue/base.py (1)
IssueUserDisplayPropertyEndpoint(718-735)
apps/api/plane/api/views/project.py (2)
apps/api/plane/db/models/project.py (1)
ProjectUserProperty(317-348)apps/api/plane/app/views/project/base.py (3)
create(238-299)create(501-508)create(578-606)
apps/api/plane/db/migrations/0114_auto_20251201_0857.py (2)
apps/api/plane/db/models/project.py (2)
ProjectMember(191-235)ProjectUserProperty(317-348)apps/api/plane/db/migrations/0113_alter_issueuserproperty_table.py (1)
Migration(8-38)
apps/api/plane/app/views/issue/base.py (1)
apps/api/plane/db/models/project.py (1)
ProjectUserProperty(317-348)
apps/api/plane/tests/contract/app/test_project_app.py (1)
apps/api/plane/db/models/project.py (1)
ProjectUserProperty(317-348)
apps/api/plane/bgtasks/workspace_seed_task.py (1)
apps/api/plane/db/models/project.py (1)
ProjectUserProperty(317-348)
apps/api/plane/db/models/project.py (1)
apps/api/plane/db/models/issue.py (3)
get_default_filters(43-54)get_default_display_filters(57-66)get_default_display_properties(69-84)
apps/api/plane/app/views/project/base.py (2)
apps/api/plane/db/models/project.py (1)
ProjectUserProperty(317-348)apps/api/plane/app/views/project/member.py (1)
create(42-150)
apps/api/plane/app/views/project/invite.py (1)
apps/api/plane/db/models/project.py (1)
ProjectUserProperty(317-348)
apps/api/plane/db/migrations/0113_alter_issueuserproperty_table.py (2)
apps/api/plane/db/migrations/0114_auto_20251201_0857.py (1)
Migration(31-42)apps/api/plane/db/models/project.py (1)
get_default_preferences(60-61)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Agent
- GitHub Check: Cursor Bugbot
- GitHub Check: Lint API
- GitHub Check: Analyze (javascript)
🔇 Additional comments (22)
apps/api/plane/app/urls/issue.py (1)
211-217: Comment rename correctly reflects underlying modelThe updated
## ProjectUserPropertycomments around theuser-properties/route align with the new model name and keep the URL behavior unchanged.apps/api/plane/db/models/__init__.py (1)
48-56: Public model surface correctly switched toProjectUserPropertyExporting
ProjectUserPropertyfrom.project(withIssueUserPropertyno longer present) correctly aligns the public API with the new model.apps/api/plane/app/serializers/issue.py (2)
16-41: Model import updated correctly toProjectUserPropertySwitching the import in
plane.db.modelsfrom the old property model toProjectUserPropertykeeps this serializer file aligned with the new model layout.
349-353:IssueUserPropertySerializernow bound toProjectUserPropertyPointing the serializer’s
Meta.modeltoProjectUserPropertymatches the refactor and stays compatible with existing views that use this serializer. Theread_only_fieldslist still matches the new model’s base fields, so no further changes seem necessary.apps/api/plane/app/views/project/base.py (1)
24-37: Project creation flow now correctly seedsProjectUserPropertyImporting and creating
ProjectUserPropertyfor the creator (and distinct project lead) keeps per-user project properties in sync with the new model and respects the unique (user, project) constraint. The conditional onproject_leadstill prevents duplicate records for the same user.Also applies to: 245-266
apps/api/plane/db/management/commands/create_project_member.py (1)
6-12: Management command now seedsProjectUserPropertyinstead ofIssueUserPropertyUpdating the import and using
ProjectUserProperty.objects.get_or_create(user=user, project=project)keeps this command aligned with the new model while preserving idempotent behavior.Also applies to: 69-71
apps/api/plane/tests/contract/app/test_project_app.py (1)
6-13: Tests correctly updated to assertProjectUserPropertycreationSwitching the import and expectations from
IssueUserPropertytoProjectUserPropertymatches the refactor: you now assert a property for the creator and two properties when a distinct project lead is set, which aligns with the updated create flow.Also applies to: 80-87, 116-121
apps/api/plane/bgtasks/workspace_seed_task.py (2)
23-23: LGTM!The import correctly references the renamed
ProjectUserPropertymodel.
121-160: LGTM!The bulk creation correctly uses
ProjectUserPropertyinstead ofIssueUserProperty. The new fieldspreferencesandsort_orderare not explicitly set, which is acceptable as they have sensible defaults defined in the model.apps/api/plane/app/views/issue/base.py (3)
54-54: LGTM!Import correctly updated to
ProjectUserProperty.
720-729: LGTM!The
patchmethod correctly usesProjectUserProperty.objects.get()and accesses fields that exist in the new model definition. Note that the serializer is still namedIssueUserPropertySerializer, which appears to be intentional as it's not part of this refactor's scope.
732-735: LGTM!The
getmethod correctly usesProjectUserProperty.objects.get_or_create()with appropriate fields matching the model's unique constraints.apps/api/plane/app/views/project/member.py (3)
16-16: LGTM!Import statement correctly includes
ProjectUserProperty.
121-127: LGTM!
ProjectUserPropertyinstantiation correctly sets the required fields. The new fieldspreferencesandsort_orderwill use their default values, consistent with the pattern used elsewhere in the codebase.
132-132: LGTM!
bulk_createcorrectly usesProjectUserPropertywith appropriate parameters includingignore_conflicts=Trueto handle the unique constraint on user and project.apps/api/plane/api/views/project.py (3)
21-21: LGTM!Import correctly updated to
ProjectUserProperty.
220-220: LGTM!
ProjectUserPropertycreation for the requesting user correctly setsproject_idanduser. The new fields will use their default values.
231-234: LGTM!
ProjectUserPropertycreation for the project lead is correctly implemented with appropriate conditional logic. Usinguser_idas a keyword argument is valid.apps/api/plane/db/migrations/0114_auto_20251201_0857.py (2)
5-28: LGTM!The data migration correctly copies
preferencesandsort_orderfromProjectMembertoProjectUserPropertyfor matching user-project pairs. The implementation is efficient using dictionary-based lookups and bulk updates with an appropriate batch size.
31-42: LGTM!The migration structure is correct with appropriate dependency on the previous migration and a no-op reverse operation, which is standard for data migrations.
apps/api/plane/app/views/project/invite.py (2)
27-27: LGTM!Import correctly updated to
ProjectUserProperty.
163-174: LGTM!The bulk creation of
ProjectUserPropertyis correctly implemented with appropriate fields andignore_conflicts=Trueto handle the unique constraint on user and project.
|
@pablohashescobar Update the |
…date related fields and constraints
There was a problem hiding this comment.
Pull request overview
This PR refactors the IssueUserProperty model to ProjectUserProperty, reflecting its broader scope beyond issue-specific properties. The refactoring includes adding preferences and sort_order fields migrated from ProjectMember, and also adds allowed_rate_limit to the APIToken model.
Key changes:
- Renamed
IssueUserPropertytoProjectUserPropertyand moved fromissue.pytoproject.py - Added
preferencesandsort_orderfields toProjectUserProperty, populated via data migration fromProjectMember - Updated all references across views, serializers, tests, management commands, and background tasks
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| apps/api/plane/db/models/issue.py | Removed IssueUserProperty model definition |
| apps/api/plane/db/models/project.py | Added ProjectUserProperty model with additional fields |
| apps/api/plane/db/models/init.py | Updated import from IssueUserProperty to ProjectUserProperty |
| apps/api/plane/db/models/api.py | Added allowed_rate_limit field to APIToken |
| apps/api/plane/db/migrations/0113_alter_issueuserproperty_table.py | Migration to rename model, alter table, and add new fields |
| apps/api/plane/db/migrations/0114_alter_projectuserproperty_options_and_more.py | Data migration from ProjectMember to ProjectUserProperty |
| apps/api/plane/app/serializers/issue.py | Updated IssueUserPropertySerializer to use ProjectUserProperty model |
| apps/api/plane/app/views/issue/base.py | Updated references to use ProjectUserProperty |
| apps/api/plane/app/views/project/base.py | Updated project creation to use ProjectUserProperty |
| apps/api/plane/app/views/project/member.py | Updated member addition to use ProjectUserProperty |
| apps/api/plane/app/views/project/invite.py | Updated invite acceptance to use ProjectUserProperty |
| apps/api/plane/api/views/project.py | Updated API project creation to use ProjectUserProperty |
| apps/api/plane/bgtasks/workspace_seed_task.py | Updated seed task to use ProjectUserProperty |
| apps/api/plane/db/management/commands/create_project_member.py | Updated management command to use ProjectUserProperty |
| apps/api/plane/app/urls/issue.py | Updated comments to reflect rename |
| apps/api/plane/tests/contract/app/test_project_app.py | Updated tests to assert ProjectUserProperty creation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…property-migrations
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/api/plane/db/migrations/0113_alter_issueuserproperty_table.py(1 hunks)apps/api/plane/db/migrations/0114_alter_projectuserproperty_options_and_more.py(1 hunks)apps/api/plane/db/models/project.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/plane/db/models/project.py
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: NarayanBavisetti
Repo: makeplane/plane PR: 7460
File: apps/api/plane/app/serializers/draft.py:112-122
Timestamp: 2025-07-23T18:18:06.875Z
Learning: In the Plane codebase serializers, workspace_id is not consistently passed in serializer context, so parent issue validation in DraftIssueCreateSerializer only checks project_id rather than both workspace_id and project_id. The existing project member authentication system already validates that users can only access projects they belong to, providing sufficient security without risking breaking functionality by adding workspace_id validation where the context might not be available.
🧬 Code graph analysis (2)
apps/api/plane/db/migrations/0114_alter_projectuserproperty_options_and_more.py (2)
apps/api/plane/db/models/project.py (2)
ProjectMember(191-235)ProjectUserProperty(317-348)apps/api/plane/db/migrations/0113_alter_issueuserproperty_table.py (1)
Migration(8-54)
apps/api/plane/db/migrations/0113_alter_issueuserproperty_table.py (2)
apps/api/plane/db/migrations/0114_alter_projectuserproperty_options_and_more.py (1)
Migration(31-39)apps/api/plane/db/models/project.py (1)
get_default_preferences(60-61)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: CodeQL analysis (python)
- GitHub Check: Agent
- GitHub Check: Cursor Bugbot
apps/api/plane/db/migrations/0114_alter_projectuserproperty_options_and_more.py
Show resolved
Hide resolved
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on December 20
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
…ertyEndpoint to ProjectUserPropertySerializer and ProjectUserDisplayPropertyEndpoint, updating all related references
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/api/plane/app/serializers/__init__.py(1 hunks)apps/api/plane/app/serializers/issue.py(2 hunks)apps/api/plane/app/urls/issue.py(2 hunks)apps/api/plane/app/views/__init__.py(1 hunks)apps/api/plane/app/views/issue/base.py(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/api/plane/app/serializers/issue.py
- apps/api/plane/app/views/issue/base.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-23T18:18:06.875Z
Learnt from: NarayanBavisetti
Repo: makeplane/plane PR: 7460
File: apps/api/plane/app/serializers/draft.py:112-122
Timestamp: 2025-07-23T18:18:06.875Z
Learning: In the Plane codebase serializers, workspace_id is not consistently passed in serializer context, so parent issue validation in DraftIssueCreateSerializer only checks project_id rather than both workspace_id and project_id. The existing project member authentication system already validates that users can only access projects they belong to, providing sufficient security without risking breaking functionality by adding workspace_id validation where the context might not be available.
Applied to files:
apps/api/plane/app/serializers/__init__.py
🧬 Code graph analysis (3)
apps/api/plane/app/serializers/__init__.py (1)
apps/api/plane/app/serializers/issue.py (1)
ProjectUserPropertySerializer(349-353)
apps/api/plane/app/views/__init__.py (1)
apps/api/plane/app/views/issue/base.py (1)
ProjectUserDisplayPropertyEndpoint(718-735)
apps/api/plane/app/urls/issue.py (1)
apps/api/plane/app/views/issue/base.py (1)
ProjectUserDisplayPropertyEndpoint(718-735)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (2)
apps/api/plane/app/serializers/__init__.py (1)
55-55: LGTM!The serializer rename correctly reflects the model rename from
IssueUserPropertytoProjectUserProperty.apps/api/plane/app/views/__init__.py (1)
118-118: LGTM!The public export correctly reflects the endpoint rename from
IssueUserDisplayPropertyEndpointtoProjectUserDisplayPropertyEndpoint, maintaining consistency with the model rename.
…perties by creating new entries and improve response handling
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/api/plane/app/views/issue/base.py (1)
718-745: Endpoint now supports updating new fields; consider minor cleanupsThe renamed
ProjectUserDisplayPropertyEndpointnow usesProjectUserPropertySerializerwithpartial=True, sopreferencesandsort_order(and other writable fields) can be updated via PATCH, which addresses the earlier review concern. You could optionally:
- Replace the explicit try/except +
createinpatchwith a singleget_or_createcall (to mirrorgetand reduce duplication).- Rename
issue_propertytoproject_user_propertyoruser_propertyfor clarity post‑refactor.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/api/plane/app/views/issue/base.py(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-23T18:18:06.875Z
Learnt from: NarayanBavisetti
Repo: makeplane/plane PR: 7460
File: apps/api/plane/app/serializers/draft.py:112-122
Timestamp: 2025-07-23T18:18:06.875Z
Learning: In the Plane codebase serializers, workspace_id is not consistently passed in serializer context, so parent issue validation in DraftIssueCreateSerializer only checks project_id rather than both workspace_id and project_id. The existing project member authentication system already validates that users can only access projects they belong to, providing sufficient security without risking breaking functionality by adding workspace_id validation where the context might not be available.
Applied to files:
apps/api/plane/app/views/issue/base.py
🧬 Code graph analysis (1)
apps/api/plane/app/views/issue/base.py (2)
apps/api/plane/app/serializers/issue.py (3)
ProjectUserPropertySerializer(349-353)create(194-269)create(580-583)apps/api/plane/db/models/project.py (1)
ProjectUserProperty(317-348)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Cursor Bugbot
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
apps/api/plane/app/views/issue/base.py (1)
37-38: Imports correctly updated to ProjectUserProperty model/serializerThe imports for
ProjectUserPropertySerializerandProjectUserPropertyare consistent with the refactor and are used appropriately in this module; no issues from this change set.Also applies to: 54-59
…emove workspace association
apps/api/plane/db/migrations/0114_alter_projectuserproperty_options_and_more.py
Show resolved
Hide resolved
…y excluding bot users from workspace removal
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/api/plane/db/migrations/0114_alter_projectuserproperty_options_and_more.py (1)
20-25: Add None checks before copying ProjectMember fields.The concern raised in previous review remains unresolved: older ProjectMember records may have
Noneforpreferencesandsort_order(migration 0110 did not backfill), which would overwrite the defaults already set in migration 0113. Add conditional checks to preserve existing defaults when source fields are None.
🧹 Nitpick comments (1)
apps/api/plane/db/migrations/0114_alter_projectuserproperty_options_and_more.py (1)
31-39: Cursor bot comment is a false positive; the migration logic is correct.The previous concern about
user__is_bot=Falsein anupdate()call is incorrect. Line 35 usesuser__is_bot=Falsein the.filter()clause (which supports related lookups), while line 36 only updatesworkspace_id(a direct field onAPIToken). This is valid Django ORM usage.The migration correctly clears workspace associations for non-service, non-bot tokens as intended.
Minor formatting note: Line 37 has an unnecessary blank line, and line 39 has an unnecessary return statement, but these are purely stylistic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/api/plane/db/migrations/0114_alter_projectuserproperty_options_and_more.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/api/plane/db/migrations/0114_alter_projectuserproperty_options_and_more.py (3)
apps/api/plane/db/models/project.py (1)
ProjectUserProperty(317-348)apps/api/plane/db/models/api.py (1)
APIToken(19-44)apps/api/plane/db/migrations/0113_alter_issueuserproperty_table.py (1)
Migration(8-55)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/api/plane/api/views/project.py (1)
18-30: Remove unused importProjectUserProperty.
ProjectUserPropertyis imported but never used in this file. The explicit creation calls were removed becauseProjectMember.save()now automatically creates the associatedProjectUserPropertyrecord.🔎 Suggested fix
from plane.db.models import ( Cycle, Intake, - ProjectUserProperty, Module, Project, DeployBoard,apps/api/plane/app/views/project/member.py (1)
57-59: Misleading variable name after refactoring.The variable
bulk_issue_propsnow holdsProjectUserPropertyinstances, not issue properties. Consider renaming it to align with the refactored model name.🔎 Suggested fix
# Initialize the bulk arrays bulk_project_members = [] - bulk_issue_props = [] + bulk_project_user_props = []Also update references at lines 120 and 132:
- bulk_issue_props.append( + bulk_project_user_props.append(- _ = ProjectUserProperty.objects.bulk_create(bulk_issue_props, batch_size=10, ignore_conflicts=True) + _ = ProjectUserProperty.objects.bulk_create(bulk_project_user_props, batch_size=10, ignore_conflicts=True)apps/api/plane/db/models/project.py (1)
207-223: Consider transaction safety forProjectUserPropertycreation.The
ProjectUserPropertyis created beforesuper().save()is called. If the parent save fails (e.g., due to a unique constraint violation), the createdProjectUserPropertyrecord becomes orphaned.Consider wrapping the creation and save in an atomic transaction, or moving the creation after the successful save:
🔎 Option 1: Use atomic transaction
+from django.db import transaction + def save(self, *args, **kwargs): if self._state.adding and self.member: - # Get the minimum sort_order for this member in the workspace - min_sort_order_result = ProjectUserProperty.objects.filter( - workspace_id=self.project.workspace_id, user=self.member - ).aggregate(min_sort_order=models.Min("sort_order")) - min_sort_order = min_sort_order_result.get("min_sort_order") - - # create project user property with project sort order - ProjectUserProperty.objects.create( - workspace_id=self.project.workspace_id, - project=self.project, - user=self.member, - sort_order=(min_sort_order - 10000 if min_sort_order is not None else 65535), - ) - - super(ProjectMember, self).save(*args, **kwargs) + with transaction.atomic(): + min_sort_order_result = ProjectUserProperty.objects.filter( + workspace_id=self.project.workspace_id, user=self.member + ).aggregate(min_sort_order=models.Min("sort_order")) + min_sort_order = min_sort_order_result.get("min_sort_order") + + ProjectUserProperty.objects.create( + workspace_id=self.project.workspace_id, + project=self.project, + user=self.member, + sort_order=(min_sort_order - 10000 if min_sort_order is not None else 65535), + ) + super(ProjectMember, self).save(*args, **kwargs) + else: + super(ProjectMember, self).save(*args, **kwargs)
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/api/plane/api/views/project.py(1 hunks)apps/api/plane/app/views/project/base.py(2 hunks)apps/api/plane/app/views/project/invite.py(3 hunks)apps/api/plane/app/views/project/member.py(3 hunks)apps/api/plane/db/management/commands/create_project_member.py(2 hunks)apps/api/plane/db/models/project.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/plane/app/views/project/base.py
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: NarayanBavisetti
Repo: makeplane/plane PR: 7460
File: apps/api/plane/app/serializers/draft.py:112-122
Timestamp: 2025-07-23T18:18:06.875Z
Learning: In the Plane codebase serializers, workspace_id is not consistently passed in serializer context, so parent issue validation in DraftIssueCreateSerializer only checks project_id rather than both workspace_id and project_id. The existing project member authentication system already validates that users can only access projects they belong to, providing sufficient security without risking breaking functionality by adding workspace_id validation where the context might not be available.
📚 Learning: 2025-07-23T18:18:06.875Z
Learnt from: NarayanBavisetti
Repo: makeplane/plane PR: 7460
File: apps/api/plane/app/serializers/draft.py:112-122
Timestamp: 2025-07-23T18:18:06.875Z
Learning: In the Plane codebase serializers, workspace_id is not consistently passed in serializer context, so parent issue validation in DraftIssueCreateSerializer only checks project_id rather than both workspace_id and project_id. The existing project member authentication system already validates that users can only access projects they belong to, providing sufficient security without risking breaking functionality by adding workspace_id validation where the context might not be available.
Applied to files:
apps/api/plane/app/views/project/invite.pyapps/api/plane/app/views/project/member.pyapps/api/plane/api/views/project.py
📚 Learning: 2025-10-29T09:17:54.815Z
Learnt from: NarayanBavisetti
Repo: makeplane/plane PR: 7905
File: apps/api/plane/app/views/search/base.py:241-276
Timestamp: 2025-10-29T09:17:54.815Z
Learning: In apps/api/plane/app/views/search/base.py, the `filter_intakes` method uses `Issue.objects` (base manager) instead of `Issue.issue_objects` (custom manager) because the custom manager filters out all intake statuses, which would prevent querying pending and snoozed intake issues.
Applied to files:
apps/api/plane/api/views/project.py
🧬 Code graph analysis (4)
apps/api/plane/db/management/commands/create_project_member.py (2)
apps/api/plane/db/models/project.py (2)
ProjectUserProperty(323-354)ProjectMember(191-241)apps/api/plane/app/views/project/member.py (1)
create(43-150)
apps/api/plane/db/models/project.py (1)
apps/api/plane/app/views/project/member.py (1)
create(43-150)
apps/api/plane/app/views/project/invite.py (1)
apps/api/plane/db/models/project.py (1)
ProjectUserProperty(323-354)
apps/api/plane/api/views/project.py (1)
apps/api/plane/db/models/project.py (1)
ProjectUserProperty(323-354)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Cursor Bugbot
- GitHub Check: Build packages
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (5)
apps/api/plane/db/management/commands/create_project_member.py (1)
51-61: LGTM - Correct handling for both update and create paths.The logic correctly handles both scenarios:
- Update path (lines 53-55): Updates existing member, then
get_or_createensuresProjectUserPropertyexists- Create path (line 58):
ProjectMember.save()auto-createsProjectUserProperty, andget_or_createsafely retrieves the existing recordapps/api/plane/app/views/project/member.py (1)
93-127: LGTM - Sort order computation is correct.The min_sort_order lookup from
ProjectUserPropertyand the calculationmin_sort_order - 10000is consistent with the pattern inProjectMember.save(). Usingbulk_createwithignore_conflicts=Truesafely handles cases where records already exist.apps/api/plane/db/models/project.py (1)
323-354: LGTM - Model definition is correct after previous review fixes.The
ProjectUserPropertymodel correctly defines:
- Proper
verbose_name/verbose_name_pluralas "Project User Property"- Correct
related_name="project_property_user"- Appropriate constraint naming
The in-class import on line 324 is an unconventional but valid pattern to avoid circular imports between
project.pyandissue.py.apps/api/plane/app/views/project/invite.py (2)
163-174: LGTM - ExplicitProjectUserPropertycreation is necessary forbulk_create.Since
bulk_createbypasses the model'ssave()method, the explicitProjectUserPropertycreation here is correct. Theignore_conflicts=Truesafely handles any duplicates.
220-230: LGTM -ProjectUserPropertyis auto-created viasave().Using
ProjectMember.objects.create()invokes thesave()method, which automatically creates the correspondingProjectUserPropertyrecord. The change fromworkspace_idtoproject_idis correct sinceProjectBaseModel.save()derives the workspace from the project.
| ) | ||
| # Also create the issue property for the user | ||
| _ = IssueUserProperty.objects.create(project_id=serializer.data["id"], user=request.user) | ||
| _ = ProjectUserProperty.objects.create(project_id=serializer.data["id"], user=request.user) |
There was a problem hiding this comment.
Bug: Duplicate ProjectUserProperty creation causes IntegrityError
The ProjectMember.save() method now automatically creates a ProjectUserProperty when a new member is added. However, line 259 still explicitly calls ProjectUserProperty.objects.create() after ProjectMember.objects.create(). This results in attempting to create two ProjectUserProperty records for the same user and project, which violates the unique constraint and will raise an IntegrityError. The explicit creation at line 259 needs to be removed, similar to how it was correctly removed in apps/api/plane/api/views/project.py.
Description
IssueUserPropertytoProjectUserPropertysort_orderandpreferencestoProjectUserPropertyProjectMemberstoProjectUserPropertyIssueUserPropertytoProjectUserPropertyallowed_rate_limittoAPITokensType of Change
Test Scenarios
References
WEB-5537
Note
Renames
IssueUserPropertytoProjectUserPropertyacross backend, addspreferencesandsort_orderwith data migrations, updates related endpoints and member flows, and addsallowed_rate_limittoAPIToken.IssueUserProperty→ProjectUserProperty(table toproject_user_properties), update relations/constraints and serializer names.preferencesandsort_ordertoProjectUserProperty; migrate values fromProjectMember(bulk update).APIToken.allowed_rate_limitand migrate existing non-service tokens to nullworkspace.IssueUserPropertymodel; exposeProjectUserPropertyfromdb.models.project.IssueUserDisplayPropertyEndpointwithProjectUserDisplayPropertyEndpointatworkspaces/<slug>/projects/<project_id>/user-properties/(partial update via serializer; creates if missing).ProjectUserProperty.ProjectUserPropertyrecords; computesort_orderusing member’s workspace min where applicable.ProjectMember.savenow auto-creates correspondingProjectUserPropertywith derivedsort_order.create_project_membernow targetsProjectUserProperty.ProjectUserPropertyfor project members.Written by Cursor Bugbot for commit d75efb3. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.