Skip to content

Conversation

@sangeethailango
Copy link
Member

@sangeethailango sangeethailango commented Jan 12, 2026

Description

This PR will add validation for special characters in project names and identifiers.
The following characters are forbidden now.

# & + , : ; $ ^ } { * = ? @ # | ' < > . ( ) % !

Type of Change

  • Improvement (change that would cause existing functionality to not work as expected)

Summary by CodeRabbit

  • Bug Fixes
    • Validation now rejects project names and identifiers containing disallowed special characters during creation and updates.
    • Consistent validation applied across all project serializers, backed by a centralized pattern to prevent invalid characters.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 12, 2026

📝 Walkthrough

Walkthrough

Added a class-level regex constant to Project and integrated forbidden-character checks using Project.FORBIDDEN_IDENTIFIER_CHARS_PATTERN into project serializers to reject project names and identifiers containing specified special characters during create, update, and general validation.

Changes

Cohort / File(s) Summary
Model constant
apps/api/plane/db/models/project.py
Added Project.FORBIDDEN_IDENTIFIER_CHARS_PATTERN (regex) as a class-level constant.
API serializers
apps/api/plane/api/serializers/project.py
Imported re and enforced forbidden-character validation using Project.FORBIDDEN_IDENTIFIER_CHARS_PATTERN in ProjectCreateSerializer.validate, ProjectCreateSerializer.update, ProjectUpdateSerializer.update, and ProjectSerializer.validate; raises ValidationError when name/identifier match pattern.
App serializers
apps/api/plane/app/serializers/project.py
Imported re and added checks in validate_name and validate_identifier using Project.FORBIDDEN_IDENTIFIER_CHARS_PATTERN; raises ValidationError for forbidden characters.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hop through code with joyful cheer,
Regex fences keep the names sincere,
No prickly chars in ID or title,
Clean projects march, neat and vital.
A tiny hop toward safer art.

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The PR title clearly and concisely summarizes the main change: adding validation for special characters in project names and identifiers.
Description check ✅ Passed The description includes the main change details and type of change, but lacks test scenarios, screenshots/media, and references sections that are part of the template.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@makeplane
Copy link

makeplane bot commented Jan 12, 2026

Linked to Plane Work Item(s)

This comment was auto-generated by Plane

@sangeethailango sangeethailango marked this pull request as ready for review January 12, 2026 09:39
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Fix all issues with AI agents
In @apps/api/plane/api/serializers/project.py:
- Around line 235-244: In the validate method the variables project_name and
project_identifier are booleans because of the accidental "is not None" check,
causing re.match to receive a boolean; change assignment to capture the actual
values (e.g., project_name = data.get("name") and project_identifier =
data.get("identifier")), then guard with truthiness or explicit None checks
before calling re.match(FORBIDDEN_NAME_CHARS_PATTERN, project_name) /
re.match(..., project_identifier) and raise serializers.ValidationError with the
existing messages when a match is found.
- Around line 176-185: In the update method of the serializer, you are passing
booleans into re.match because project_name and project_identifier are set to
the result of a comparison; change them to the actual values from validated_data
(e.g., project_name = validated_data.get("name", None) and project_identifier =
validated_data.get("identifier", None")) and then guard the regex checks with
truthy checks like if project_name and re.match(FORBIDDEN_NAME_CHARS_PATTERN,
project_name): (same for project_identifier) so re.match receives the string not
a boolean.
- Around line 108-117: The validate methods are assigning booleans (via "is not
None") to project_name/project_identifier and passing those to re.match; change
each assignment to capture the actual string (e.g., project_name =
data.get("name")) and then guard the regex with an existence check (e.g., if
project_name is not None and re.match(FORBIDDEN_NAME_CHARS_PATTERN,
project_name)): apply the same fix in ProjectUpdateSerializer.update and
ProjectSerializer.validate by replacing the "is not None" boolean assignments
with the actual value retrieval and checking that value before calling re.match.
🧹 Nitpick comments (3)
apps/api/plane/app/serializers/project.py (2)

4-5: Minor: Standard library imports should come before third-party imports.

Per PEP 8, the import order should be: standard library → third-party → local. Consider moving import re above the rest_framework import.

Suggested reordering
+# Python imports
+import re
+
 # Third party imports
 from rest_framework import serializers

-# Python imports
-import re
-

25-28: Consider centralizing the FORBIDDEN_NAME_CHARS_PATTERN constant.

This constant is duplicated in apps/api/plane/api/serializers/project.py. To maintain DRY, consider defining it in a shared module (e.g., a constants.py or utility file) and importing it in both serializer files. This ensures consistency if the pattern needs to change in the future.

apps/api/plane/api/serializers/project.py (1)

5-7: Minor: Import ordering.

Same as the other file - consider moving import re before third-party imports per PEP 8.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8399f64 and aa582b7.

📒 Files selected for processing (2)
  • apps/api/plane/api/serializers/project.py
  • apps/api/plane/app/serializers/project.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/api/serializers/project.py
  • apps/api/plane/app/serializers/project.py
📚 Learning: 2025-12-23T14:18:32.899Z
Learnt from: dheeru0198
Repo: makeplane/plane PR: 8339
File: apps/api/plane/db/models/api.py:35-35
Timestamp: 2025-12-23T14:18:32.899Z
Learning: Django REST Framework rate limit strings are flexible: only the first character of the time unit matters. Acceptable formats include: "60/s", "60/sec", "60/second" (all equivalent), "60/m", "60/min", "60/minute" (all equivalent), "60/h", "60/hr", "60/hour" (all equivalent), and "60/d", "60/day" (all equivalent). Abbreviations like "min" are valid and do not need to be changed to "minute". Apply this guidance to any Python files in the project that configure DRF throttling rules.

Applied to files:

  • apps/api/plane/api/serializers/project.py
  • apps/api/plane/app/serializers/project.py
🔇 Additional comments (3)
apps/api/plane/app/serializers/project.py (2)

40-57: LGTM!

The validation logic correctly uses the name parameter (which is the field value in DRF field-level validators) with re.match. The error message is clear.


59-76: LGTM!

Consistent with validate_name, the validation correctly applies the forbidden character check to the identifier.

apps/api/plane/api/serializers/project.py (1)

17-20: Duplicate constant definition.

As noted in the other file, this constant should be centralized to avoid duplication.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
apps/api/plane/api/serializers/project.py (1)

176-184: Redundant validation - already handled by inherited validate().

ProjectUpdateSerializer extends ProjectCreateSerializer, which already validates forbidden characters in validate() (lines 108-116). Since DRF calls validate() before update(), this validation runs twice.

♻️ Suggested fix: Remove redundant validation
     def update(self, instance, validated_data):
-        project_name = validated_data.get("name", None)
-        project_identifier = validated_data.get("identifier", None)
-
-        if project_name is not None and re.match(FORBIDDEN_NAME_CHARS_PATTERN, project_name):
-            raise serializers.ValidationError("Project name cannot contain special characters.")
-
-        if project_identifier is not None and re.match(FORBIDDEN_NAME_CHARS_PATTERN, project_identifier):
-            raise serializers.ValidationError("Project identifier cannot contain special characters.")
-
         """Update a project"""
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between aa582b7 and 818bec1.

📒 Files selected for processing (1)
  • apps/api/plane/api/serializers/project.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/api/serializers/project.py
📚 Learning: 2025-12-23T14:18:32.899Z
Learnt from: dheeru0198
Repo: makeplane/plane PR: 8339
File: apps/api/plane/db/models/api.py:35-35
Timestamp: 2025-12-23T14:18:32.899Z
Learning: Django REST Framework rate limit strings are flexible: only the first character of the time unit matters. Acceptable formats include: "60/s", "60/sec", "60/second" (all equivalent), "60/m", "60/min", "60/minute" (all equivalent), "60/h", "60/hr", "60/hour" (all equivalent), and "60/d", "60/day" (all equivalent). Abbreviations like "min" are valid and do not need to be changed to "minute". Apply this guidance to any Python files in the project that configure DRF throttling rules.

Applied to files:

  • apps/api/plane/api/serializers/project.py
⏰ 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: Analyze (javascript)
🔇 Additional comments (3)
apps/api/plane/api/serializers/project.py (3)

17-20: LGTM!

The regex pattern correctly matches strings containing any of the forbidden special characters. The character class is properly constructed with all characters behaving as literals within [...].


108-116: LGTM!

The validation logic correctly guards against None values before applying the regex check. The early validation placement is appropriate to fail fast before more expensive database queries.


235-243: LGTM!

Since ProjectSerializer extends BaseSerializer directly (not ProjectCreateSerializer), it correctly implements its own forbidden character validation. The implementation is consistent with the other serializers.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In @apps/api/plane/db/models/project.py:
- Around line 139-140: The regex constant FORBIDDEN_IDENTIFIER_CHARS_PATTERN
currently forbids the hyphen character (dash) which breaks existing
examples/tests like test_issue_recent_visit.py ("test-project") and OpenAPI
examples ("new-project" / "updated-project"); update the pattern in
FORBIDDEN_IDENTIFIER_CHARS_PATTERN by removing the hyphen from the character
class so hyphenated identifiers are allowed (adjust the character class
ordering/escaping if needed), then run relevant serializer validation tests and
update any examples only if you intentionally want to disallow hyphens.
🧹 Nitpick comments (4)
apps/api/plane/app/serializers/project.py (1)

4-5: Import order: Python standard library imports should precede third-party imports.

Per PEP 8 conventions, standard library imports (re) should come before third-party imports (rest_framework).

Suggested fix
+# Python imports
+import re
+
 # Third party imports
 from rest_framework import serializers
 
-# Python imports
-import re
-
 # Module imports
apps/api/plane/api/serializers/project.py (3)

5-7: Import order: Python standard library imports should precede third-party imports.

Same as the other serializers file - standard library imports should come first per PEP 8.

Suggested fix
+# Python imports
+import re
+
 # Third party imports
 import random
 from rest_framework import serializers
-
-
-# Python imports
-import re
-
 
 # Module imports

171-179: Redundant validation: validate() already checks forbidden characters before update() is called.

ProjectUpdateSerializer inherits validate() from ProjectCreateSerializer, which already performs the forbidden character checks. DRF's serializer flow calls validate() before update(), so these checks will run twice.

Additionally, the docstring at line 181 is misplaced after the validation code.

Suggested fix - remove redundant validation
     def update(self, instance, validated_data):
-        project_name = validated_data.get("name", None)
-        project_identifier = validated_data.get("identifier", None)
-
-        if project_name is not None and re.match(Project.FORBIDDEN_IDENTIFIER_CHARS_PATTERN, project_name):
-            raise serializers.ValidationError("Project name cannot contain special characters.")
-
-        if project_identifier is not None and re.match(Project.FORBIDDEN_IDENTIFIER_CHARS_PATTERN, project_identifier):
-            raise serializers.ValidationError("Project identifier cannot contain special characters.")
-
         """Update a project"""
         if (
             validated_data.get("default_state", None) is not None

103-111: Consider extracting forbidden character validation to a reusable helper.

The same validation pattern is repeated across multiple serializers (with slight variations in error messages). A helper function on the Project model or in a utility module would reduce duplication:

# In Project model or utils
@classmethod
def validate_no_forbidden_chars(cls, value, field_name):
    if value and re.match(cls.FORBIDDEN_IDENTIFIER_CHARS_PATTERN, value):
        raise ValueError(f"{field_name} cannot contain special characters")

This is optional and can be deferred if the team prefers the current explicit approach.

Also applies to: 230-238

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between db5045c and 2641227.

📒 Files selected for processing (3)
  • apps/api/plane/api/serializers/project.py
  • apps/api/plane/app/serializers/project.py
  • apps/api/plane/db/models/project.py
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-23T14:18:32.899Z
Learnt from: dheeru0198
Repo: makeplane/plane PR: 8339
File: apps/api/plane/db/models/api.py:35-35
Timestamp: 2025-12-23T14:18:32.899Z
Learning: Django REST Framework rate limit strings are flexible: only the first character of the time unit matters. Acceptable formats include: "60/s", "60/sec", "60/second" (all equivalent), "60/m", "60/min", "60/minute" (all equivalent), "60/h", "60/hr", "60/hour" (all equivalent), and "60/d", "60/day" (all equivalent). Abbreviations like "min" are valid and do not need to be changed to "minute". Apply this guidance to any Python files in the project that configure DRF throttling rules.

Applied to files:

  • apps/api/plane/db/models/project.py
  • apps/api/plane/api/serializers/project.py
  • apps/api/plane/app/serializers/project.py
📚 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/api/serializers/project.py
  • apps/api/plane/app/serializers/project.py
🧬 Code graph analysis (2)
apps/api/plane/api/serializers/project.py (1)
apps/api/plane/db/models/project.py (1)
  • Project (64-173)
apps/api/plane/app/serializers/project.py (1)
apps/api/plane/db/models/project.py (1)
  • Project (64-173)
⏰ 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: Analyze (javascript)
🔇 Additional comments (4)
apps/api/plane/app/serializers/project.py (2)

39-40: LGTM!

The validation correctly uses re.match with the centralized pattern from the model, and raises a clear error code for frontend handling.


58-59: LGTM!

Consistent validation implementation with appropriate error code.

apps/api/plane/api/serializers/project.py (2)

103-111: LGTM!

The validation logic is correctly placed in validate() which runs before create(), ensuring data is validated early in the serialization flow.


230-238: LGTM!

The validation is correctly implemented in ProjectSerializer.validate(). Since this serializer doesn't inherit from ProjectCreateSerializer, the validation logic is necessary here.

Comment on lines +139 to +140
FORBIDDEN_IDENTIFIER_CHARS_PATTERN = r"^.*[&+,:;$^}{*=?@#|'<>.()%!-].*$"

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's read the file around lines 139-140
cat -n apps/api/plane/db/models/project.py | sed -n '130,150p'

Repository: makeplane/plane

Length of output: 895


🏁 Script executed:

# Let's search for any references to this pattern or forbidden characters
rg -i "forbidden.*char|identifier" apps/api/plane/db/models/ -A 3 -B 3

Repository: makeplane/plane

Length of output: 15923


🏁 Script executed:

# Look for tests related to this pattern
fd -e py -path "*/test*" | xargs rg -l "FORBIDDEN_IDENTIFIER_CHARS_PATTERN|forbidden.*char" 2>/dev/null

Repository: makeplane/plane

Length of output: 364


🏁 Script executed:

# Search for any usage of this pattern in the codebase
rg "FORBIDDEN_IDENTIFIER_CHARS_PATTERN" -A 2 -B 2

Repository: makeplane/plane

Length of output: 3775


🏁 Script executed:

# Search for tests related to forbidden chars pattern
rg -i "forbidden|special.*character" apps/api --type py -B 3 -A 3 | head -100

Repository: makeplane/plane

Length of output: 6471


🏁 Script executed:

# Look for PR description or requirements file
find . -type f \( -name "*.md" -o -name "*.txt" -o -name "REQUIREMENTS*" -o -name "PR*" \) | xargs grep -l "forbidden.*char\|identifier.*char" 2>/dev/null

Repository: makeplane/plane

Length of output: 43


🏁 Script executed:

# Check if there are any test files that validate the pattern
fd "test.*project" -e py | xargs rg -l "FORBIDDEN_IDENTIFIER_CHARS_PATTERN\|forbidden.*char" 2>/dev/null

Repository: makeplane/plane

Length of output: 43


🏁 Script executed:

# Search for any comments or documentation about which characters are allowed/forbidden
rg -i "allow.*hyphen|disallow.*hyphen|project.*identifier.*format" apps/api --type py

Repository: makeplane/plane

Length of output: 41


🏁 Script executed:

# Search for test files in various test directories
find . -type f -name "*.py" -path "*/test*" | head -20

Repository: makeplane/plane

Length of output: 986


🏁 Script executed:

# Look for any test that validates identifier or project name validation
rg "identifier.*test\|test.*identifier\|project.*name.*special\|hyphen" --type py -B 2 -A 2

Repository: makeplane/plane

Length of output: 41


🏁 Script executed:

# Check the serializer file more thoroughly to understand the validation logic
cat -n apps/api/plane/api/serializers/project.py | sed -n '1,150p'

Repository: makeplane/plane

Length of output: 5720


🏁 Script executed:

# Let's check if there are any git commits or diffs related to this pattern
git log --oneline --all -S "FORBIDDEN_IDENTIFIER_CHARS_PATTERN" | head -10

Repository: makeplane/plane

Length of output: 145


🏁 Script executed:

# Check if there's any documentation or comments about allowed characters
rg "project.*identifier\|identifier.*format" apps/api/plane/api/serializers/project.py -B 5 -A 5

Repository: makeplane/plane

Length of output: 41


🏁 Script executed:

# Search for test files specifically for project validation
fd "test.*project" --type f | xargs grep -l "validate\|FORBIDDEN" 2>/dev/null

Repository: makeplane/plane

Length of output: 112


🏁 Script executed:

# Look for any validation tests or examples with hyphens
rg "hyphen\|identifier.*-\|name.*-\|My-Project\|PROJ-" apps/api/plane/tests --type py -B 2 -A 2

Repository: makeplane/plane

Length of output: 41


🏁 Script executed:

# Check the app serializers too
cat -n apps/api/plane/app/serializers/project.py | head -200

Repository: makeplane/plane

Length of output: 8124


🏁 Script executed:

# Search for any comments or docstrings that explain identifier rules
rg "identifier.*must|identifier.*should|identifier.*allow|identifier.*contain" apps/api --type py -B 1 -A 1

Repository: makeplane/plane

Length of output: 1114


🏁 Script executed:

# Check the project test file
cat -n apps/api/plane/tests/contract/app/test_project_app.py | head -200

Repository: makeplane/plane

Length of output: 8951


🏁 Script executed:

# Look for any usage examples or comments mentioning hyphen or dashes
rg "identifier.*=\|name.*=" apps/api/plane/tests --type py -B 1 -A 1 | head -50

Repository: makeplane/plane

Length of output: 41


🏁 Script executed:

# Search the entire codebase for actual project identifiers to see patterns used
rg "identifier.*[:=]\s*['\"].*['\"]" apps/api --type py | head -20

Repository: makeplane/plane

Length of output: 2192


🏁 Script executed:

# Check if there are any migration files that might document the intent
fd "migration" --type f -name "*.py" | xargs grep -l "FORBIDDEN\|identifier\|special" 2>/dev/null

Repository: makeplane/plane

Length of output: 291


🏁 Script executed:

# Let's verify if the pattern is actually enforced by testing it directly
python3 << 'EOF'
import re

pattern = r"^.*[&+,:;$^}{*=?@#|'<>.()%!-].*$"

# Test cases found in the codebase
test_cases = [
    ("test-project", True),  # Should fail if hyphen is forbidden
    ("TEST-PROJECT", True),  # Uppercase version
    ("new-project", True),
    ("updated-project", True),
    ("NPT", False),  # All caps, no special chars
    ("PWL", False),
    ("test_project", False),  # Underscore not in forbidden list
    ("testproject", False),  # No special chars
    ("test@project", True),  # @ is forbidden
    ("test#project", True),  # # is forbidden
]

print("Pattern: " + pattern)
print("\nTest Results:")
for test_str, should_match in test_cases:
    result = re.match(pattern, test_str)
    matched = result is not None
    status = "✓" if matched == should_match else "✗ MISMATCH"
    print(f"  {status} '{test_str}': matched={matched}, expected={should_match}")
EOF

Repository: makeplane/plane

Length of output: 145


🏁 Script executed:

# Check when the FORBIDDEN_IDENTIFIER_CHARS_PATTERN was added
git log --all --full-history -S "FORBIDDEN_IDENTIFIER_CHARS_PATTERN" --oneline 2>/dev/null | head -5

Repository: makeplane/plane

Length of output: 145


🏁 Script executed:

# Check when the test with "test-project" identifier was added
git log --all --full-history -S "test-project" --oneline 2>/dev/null | head -5

Repository: makeplane/plane

Length of output: 145


🏁 Script executed:

# Let's check that test file more carefully to see if it actually expects hyphens to work
cat -n apps/api/plane/tests/unit/serializers/test_issue_recent_visit.py | grep -A 10 -B 5 "test-project"

Repository: makeplane/plane

Length of output: 789


🏁 Script executed:

# Check the openapi examples more closely
cat -n apps/api/plane/utils/openapi/examples.py | grep -A 5 -B 5 "new-project\|updated-project"

Repository: makeplane/plane

Length of output: 892


🏁 Script executed:

# Run a simple Python test to validate the regex behavior
python3 << 'EOF'
import re

# The actual pattern from the code
pattern = r"^.*[&+,:;$^}{*=?@#|'<>.()%!-].*$"

# Test identifiers found in the codebase
identifiers_with_hyphens = [
    "test-project",
    "new-project", 
    "updated-project",
]

print("Testing pattern:", pattern)
print("\nIdentifiers with hyphens (found in codebase):")
for identifier in identifiers_with_hyphens:
    matches = re.match(pattern, identifier)
    print(f"  '{identifier}': MATCHES pattern = {matches is not None}")
EOF

Repository: makeplane/plane

Length of output: 280


🏁 Script executed:

# Let's check if the serializer validation is actually applied in the test by looking for error assertions
rg "PROJECT_IDENTIFIER_CANNOT_CONTAIN_SPECIAL_CHARACTERS|special.*character" apps/api/plane/tests --type py -B 5 -A 5

Repository: makeplane/plane

Length of output: 41


🏁 Script executed:

# Let's see if there's any evidence of what the PR objectives actually said
git log --all --oneline | grep -i "forbidden\|identifier\|special" | head -10

Repository: makeplane/plane

Length of output: 1028


🏁 Script executed:

# Check git history for recent changes to the pattern or related code
git log --all --oneline -- apps/api/plane/db/models/project.py | head -10

Repository: makeplane/plane

Length of output: 741


🏁 Script executed:

# Let's verify the actual behavior by checking if these hyphenated identifiers would work
# in the serializer validation context
python3 << 'EOF'
import re

# The pattern in the code
pattern = r"^.*[&+,:;$^}{*=?@#|'<>.()%!-].*$"

# Examples found in the codebase
examples = {
    "test-project": "Test file identifier",
    "new-project": "OpenAPI example",
    "updated-project": "OpenAPI example",
    "NPT": "Test data (no hyphen)",
    "TEST": "Test data (no hyphen)",
}

print("Validation Results (re.match returns True = REJECTED by serializer):")
print("=" * 70)
for identifier, source in examples.items():
    matches = bool(re.match(pattern, identifier))
    status = "REJECTED" if matches else "ALLOWED"
    print(f"{status:10} | {identifier:20} | {source}")
EOF

Repository: makeplane/plane

Length of output: 475


Hyphen - is inadvertently included in forbidden characters pattern, conflicting with codebase examples.

The regex pattern forbids hyphen, but test data (test-project in test_issue_recent_visit.py) and OpenAPI examples (new-project, updated-project) use hyphenated identifiers. These would be rejected by serializer validation due to the pattern match.

Either remove hyphen from the pattern to allow hyphenated identifiers, or update the examples to use allowed formats.

Suggested fix if hyphen should be allowed
-    FORBIDDEN_IDENTIFIER_CHARS_PATTERN = r"^.*[&+,:;$^}{*=?@#|'<>.()%!-].*$"
+    FORBIDDEN_IDENTIFIER_CHARS_PATTERN = r"^.*[&+,:;$^}{*=?@#|'<>.()%!].*$"
🤖 Prompt for AI Agents
In @apps/api/plane/db/models/project.py around lines 139 - 140, The regex
constant FORBIDDEN_IDENTIFIER_CHARS_PATTERN currently forbids the hyphen
character (dash) which breaks existing examples/tests like
test_issue_recent_visit.py ("test-project") and OpenAPI examples ("new-project"
/ "updated-project"); update the pattern in FORBIDDEN_IDENTIFIER_CHARS_PATTERN
by removing the hyphen from the character class so hyphenated identifiers are
allowed (adjust the character class ordering/escaping if needed), then run
relevant serializer validation tests and update any examples only if you
intentionally want to disallow hyphens.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants