OSIDB-4920: Fix JiraProjectFields unique constraint dupliate names#1248
OSIDB-4920: Fix JiraProjectFields unique constraint dupliate names#1248JakubFrejlach wants to merge 1 commit intomasterfrom
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe pull request introduces a Django migration that alters the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/trackers/migrations/0003_alter_jiraprojectfields_unique_together.py`:
- Around line 13-16: Migration 0003
(apps/trackers/migrations/0003_alter_jiraprojectfields_unique_together.py)
changes the unique_together on model JiraProjectFields from (project_key,
field_name) to (project_key, field_id) and needs a pre-migration
cleanup/validation step to avoid constraint violations; add a RunPython
operation in that migration (or an accompanying pre-deploy script) that queries
trackers_jiraprojectfields for duplicate (project_key, field_id) pairs, logs or
resolves duplicates (e.g., merge/remove extras or set a canonical field_id), and
only then applies migrations.AlterUniqueTogether to ensure the DB has no rows
that would break the new constraint.
In `@apps/trackers/models.py`:
- Line 15: The change to unique_together = ("project_key", "field_id") makes
lookups by field_name ambiguous; update either the storage constraint or the
query logic: either restore/expand the uniqueness (e.g., include "field_name" or
include issue-type scope) in the model where unique_together is defined, or
modify the lookup paths in apps/trackers/jira/query.py (the functions that call
.first() when resolving field_name) to detect multiple matches and fail fast
(raise a clear error) or disambiguate using issue-type context; ensure the
chosen fix references the unique_together tuple in models.py and the specific
field_name resolution code in apps/trackers/jira/query.py so lookups return a
single, deterministic field_id/allowed_values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 13063364-53aa-4b8b-b5bb-396ea060a20f
📒 Files selected for processing (3)
apps/trackers/migrations/0003_alter_jiraprojectfields_unique_together.pyapps/trackers/models.pydocs/CHANGELOG.md
| migrations.AlterUniqueTogether( | ||
| name='jiraprojectfields', | ||
| unique_together={('project_key', 'field_id')}, | ||
| ), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Run on a DB snapshot before applying migration 0003.
# Expected result: zero rows returned.
psql "$DATABASE_URL" -c "
SELECT project_key, field_id, COUNT(*) AS dup_count
FROM trackers_jiraprojectfields
GROUP BY project_key, field_id
HAVING COUNT(*) > 1
ORDER BY dup_count DESC, project_key, field_id;
"Repository: RedHatProductSecurity/osidb
Length of output: 115
🏁 Script executed:
find . -name "*jiraprojectfields*" -o -name "*migration*" | grep -E "(migration|jiraproject)" | head -20Repository: RedHatProductSecurity/osidb
Length of output: 522
🏁 Script executed:
cd apps/trackers/migrations && ls -la | grep -E "^-.*\.py$" | head -20Repository: RedHatProductSecurity/osidb
Length of output: 363
🏁 Script executed:
cat -n apps/trackers/migrations/0003_alter_jiraprojectfields_unique_together.pyRepository: RedHatProductSecurity/osidb
Length of output: 562
🏁 Script executed:
cat -n apps/trackers/migrations/0001_initial.pyRepository: RedHatProductSecurity/osidb
Length of output: 1289
🏁 Script executed:
cat -n apps/trackers/migrations/0002_jirabugissuetype.pyRepository: RedHatProductSecurity/osidb
Length of output: 760
🏁 Script executed:
find . -name "models.py" -path "*/trackers/*" -exec cat -n {} \;Repository: RedHatProductSecurity/osidb
Length of output: 1066
Add a pre-migration check or cleanup step for the constraint change.
Migration 0003 changes the unique constraint from (project_key, field_name) to (project_key, field_id) without safeguards. Since the initial schema did not enforce uniqueness on field_id alone, existing duplicate pairs could cause deployment failure. Either add a RunPython step in this migration to validate/clean duplicates, or provide an explicit pre-deployment verification command for operators:
SELECT project_key, field_id, COUNT(*) AS dup_count
FROM trackers_jiraprojectfields
GROUP BY project_key, field_id
HAVING COUNT(*) > 1;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/trackers/migrations/0003_alter_jiraprojectfields_unique_together.py`
around lines 13 - 16, Migration 0003
(apps/trackers/migrations/0003_alter_jiraprojectfields_unique_together.py)
changes the unique_together on model JiraProjectFields from (project_key,
field_name) to (project_key, field_id) and needs a pre-migration
cleanup/validation step to avoid constraint violations; add a RunPython
operation in that migration (or an accompanying pre-deploy script) that queries
trackers_jiraprojectfields for duplicate (project_key, field_id) pairs, logs or
resolves duplicates (e.g., merge/remove extras or set a canonical field_id), and
only then applies migrations.AlterUniqueTogether to ensure the DB has no rows
that would break the new constraint.
|
|
||
| class Meta: | ||
| unique_together = ("project_key", "field_name") | ||
| unique_together = ("project_key", "field_id") |
There was a problem hiding this comment.
Constraint change makes existing field_name lookups ambiguous.
Line 15 allows duplicate (project_key, field_name) rows, but query paths still resolve by field_name and take .first() (see apps/trackers/jira/query.py Lines 762-770, 530-545, and 382-390). That can return the wrong field_id/allowed_values and produce incorrect Jira payload fields.
Please add disambiguation (for example, include issue-type scope in storage/lookups) or fail fast when multiple rows match a field_name.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/trackers/models.py` at line 15, The change to unique_together =
("project_key", "field_id") makes lookups by field_name ambiguous; update either
the storage constraint or the query logic: either restore/expand the uniqueness
(e.g., include "field_name" or include issue-type scope) in the model where
unique_together is defined, or modify the lookup paths in
apps/trackers/jira/query.py (the functions that call .first() when resolving
field_name) to detect multiple matches and fail fast (raise a clear error) or
disambiguate using issue-type context; ensure the chosen fix references the
unique_together tuple in models.py and the specific field_name resolution code
in apps/trackers/jira/query.py so lookups return a single, deterministic
field_id/allowed_values.
This PR fixes issue with multiple fields in the same project having the same name which violates unique constraint.