fix: correct property PINs, addresses, and add missing properties#54
Open
chitcommit wants to merge 1 commit intomainfrom
Open
fix: correct property PINs, addresses, and add missing properties#54chitcommit wants to merge 1 commit intomainfrom
chitcommit wants to merge 1 commit intomainfrom
Conversation
Corrective migration for data errors in 0004 and 0007: - Fix Addison PIN: 14-21-307-032-1006 → 14-21-111-008-1006 - Fix Surf 504 PIN: 14-28-200-011-1042 → 14-28-122-017-1091 - Fix Surf 504 address: 550 → 559 W Surf St (per deed) - Remove wrong properties (Park Forest, Clarendon Hills) - Add correct: Surf 211 (550 W Surf #C211), Clarendon (4343 N Clarendon #1610), Medellín - Update related obligation and account metadata Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
📝 WalkthroughWalkthroughA SQL migration corrects property data by fixing tax PIN values, updating property metadata, deleting erroneous records, inserting corrected condo properties, and amending account and obligation metadata in the database. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
To use Codex here, create a Codex account and connect to github. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
migrations/0014_fix_properties.sql (2)
78-81: Metadata replacement may discard existing fields.Using
SET metadata = '{...}'::jsonbreplaces the entire metadata object, discarding any existing fields. Other updates in this migration (lines 86, 94, 98, 102) usemetadata || '{...}'::jsonbto merge instead.If intentional (e.g., the existing metadata was entirely wrong), this is fine. Otherwise, consider using merge:
Use merge operator to preserve existing metadata
UPDATE cc_accounts - SET metadata = '{"property": "559 W Surf St `#C504`"}'::jsonb + SET metadata = metadata || '{"property": "559 W Surf St `#C504`"}'::jsonb WHERE source = 'mr_cooper' AND account_name = '550 W Surf Mortgage';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@migrations/0014_fix_properties.sql` around lines 78 - 81, The UPDATE for cc_accounts where source = 'mr_cooper' AND account_name = '550 W Surf Mortgage' currently replaces the entire metadata JSON; change it to merge the new property into existing metadata (use the jsonb merge operator as used elsewhere in this migration) so existing fields are preserved—for consistency, update the SET clause for that UPDATE to merge rather than assign, or if full replacement was intentional, add a clear comment explaining why.
89-91: Same metadata replacement pattern as noted above.Lines 90 and 106 use full replacement (
=) while adjacent updates (lines 86, 94, 98, 102) use merge (||). If intentional, consider adding a comment explaining why these specific obligations need full metadata replacement.Use merge for consistency (if appropriate)
UPDATE cc_obligations - SET metadata = '{"property": "559 W Surf St `#C504`", "pin": "14-28-122-017-1091"}'::jsonb + SET metadata = metadata || '{"property": "559 W Surf St `#C504`", "pin": "14-28-122-017-1091"}'::jsonb WHERE category = 'mortgage' AND payee ILIKE '%Surf%';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@migrations/0014_fix_properties.sql` around lines 89 - 91, The UPDATE on cc_obligations that sets metadata = '{"property": "559 W Surf St `#C504`", "pin": "14-28-122-017-1091"}'::jsonb for category = 'mortgage' AND payee ILIKE '%Surf%' uses full replacement of the metadata column while nearby updates use JSON merge (metadata || ...); either change this statement to use the same merge pattern (metadata = metadata || '{"property": "...", "pin": "..."}'::jsonb) to preserve existing metadata, or add an inline SQL comment above this UPDATE explaining why a full replacement (metadata =) is intentionally required for this specific record to make the intent explicit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@migrations/0014_fix_properties.sql`:
- Around line 42-52: The HOA payee spelling is inconsistent between
cc_properties inserts (the INSERT for property_name 'Surf 211' sets hoa_payee to
'Commodore/Greenbriar Landmark Condo Association' while another record uses
'Commodore/Greenbrier Landmark Condo Association'); verify the authoritative
spelling and update the hoa_payee value in this INSERT statement (and any other
cc_properties rows) so the hoa_payee field is identical across records to avoid
data-quality issues when querying/displaying HOA information.
- Around line 66-74: The INSERT into cc_properties for 'Medellín' is
non-idempotent (tax_pin is NULL so unique constraints won’t prevent duplicates);
make it idempotent by either adding a NOT EXISTS guard around the insert (check
cc_properties for property_name = 'Medellín') or by adding a UNIQUE constraint
on property_name and converting the INSERT into an ON CONFLICT DO NOTHING using
property_name as the conflict target; update the migration to use one of these
approaches so repeated runs do not create duplicate Medellín rows.
---
Nitpick comments:
In `@migrations/0014_fix_properties.sql`:
- Around line 78-81: The UPDATE for cc_accounts where source = 'mr_cooper' AND
account_name = '550 W Surf Mortgage' currently replaces the entire metadata
JSON; change it to merge the new property into existing metadata (use the jsonb
merge operator as used elsewhere in this migration) so existing fields are
preserved—for consistency, update the SET clause for that UPDATE to merge rather
than assign, or if full replacement was intentional, add a clear comment
explaining why.
- Around line 89-91: The UPDATE on cc_obligations that sets metadata =
'{"property": "559 W Surf St `#C504`", "pin": "14-28-122-017-1091"}'::jsonb for
category = 'mortgage' AND payee ILIKE '%Surf%' uses full replacement of the
metadata column while nearby updates use JSON merge (metadata || ...); either
change this statement to use the same merge pattern (metadata = metadata ||
'{"property": "...", "pin": "..."}'::jsonb) to preserve existing metadata, or
add an inline SQL comment above this UPDATE explaining why a full replacement
(metadata =) is intentionally required for this specific record to make the
intent explicit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8d1b47cc-32be-40f1-9440-77fe37dfd1f9
📒 Files selected for processing (1)
migrations/0014_fix_properties.sql
Comment on lines
+42
to
+52
| INSERT INTO cc_properties (property_name, address, unit, tax_pin, property_type, hoa_payee, metadata) | ||
| VALUES ( | ||
| 'Surf 211', | ||
| '550 W Surf St', | ||
| '#C211', | ||
| '14-28-122-017-1180', | ||
| 'condo', | ||
| 'Commodore/Greenbriar Landmark Condo Association', | ||
| '{"purchase_price": 100000, "purchase_date": "2022-07-01", "pin": "14-28-122-017-1180", "condo_declaration": "26911238", "owner_entity": "ARIBIA LLC - CITY STUDIO"}'::jsonb | ||
| ) | ||
| ON CONFLICT (tax_pin) DO NOTHING; |
There was a problem hiding this comment.
HOA payee name spelling inconsistency.
Line 49 uses 'Commodore/Greenbriar Landmark Condo Association' (with "ia") but line 22 uses 'Commodore/Greenbrier Landmark Condo Association' (with "ie"). This inconsistency could cause data quality issues and confusion when querying or displaying HOA information.
Suggested fix: Align spelling (verify correct spelling first)
'Surf 211',
'550 W Surf St',
'#C211',
'14-28-122-017-1180',
'condo',
- 'Commodore/Greenbriar Landmark Condo Association',
+ 'Commodore/Greenbrier Landmark Condo Association',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| INSERT INTO cc_properties (property_name, address, unit, tax_pin, property_type, hoa_payee, metadata) | |
| VALUES ( | |
| 'Surf 211', | |
| '550 W Surf St', | |
| '#C211', | |
| '14-28-122-017-1180', | |
| 'condo', | |
| 'Commodore/Greenbriar Landmark Condo Association', | |
| '{"purchase_price": 100000, "purchase_date": "2022-07-01", "pin": "14-28-122-017-1180", "condo_declaration": "26911238", "owner_entity": "ARIBIA LLC - CITY STUDIO"}'::jsonb | |
| ) | |
| ON CONFLICT (tax_pin) DO NOTHING; | |
| INSERT INTO cc_properties (property_name, address, unit, tax_pin, property_type, hoa_payee, metadata) | |
| VALUES ( | |
| 'Surf 211', | |
| '550 W Surf St', | |
| '#C211', | |
| '14-28-122-017-1180', | |
| 'condo', | |
| 'Commodore/Greenbrier Landmark Condo Association', | |
| '{"purchase_price": 100000, "purchase_date": "2022-07-01", "pin": "14-28-122-017-1180", "condo_declaration": "26911238", "owner_entity": "ARIBIA LLC - CITY STUDIO"}'::jsonb | |
| ) | |
| ON CONFLICT (tax_pin) DO NOTHING; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@migrations/0014_fix_properties.sql` around lines 42 - 52, The HOA payee
spelling is inconsistent between cc_properties inserts (the INSERT for
property_name 'Surf 211' sets hoa_payee to 'Commodore/Greenbriar Landmark Condo
Association' while another record uses 'Commodore/Greenbrier Landmark Condo
Association'); verify the authoritative spelling and update the hoa_payee value
in this INSERT statement (and any other cc_properties rows) so the hoa_payee
field is identical across records to avoid data-quality issues when
querying/displaying HOA information.
Comment on lines
+66
to
+74
| -- Medellín: International property held by ARIBIA LLC | ||
| INSERT INTO cc_properties (property_name, address, unit, property_type, metadata) | ||
| VALUES ( | ||
| 'Medellín', | ||
| 'Urbanización Plaza De Colores, Carrera 76, Medellín, Colombia', | ||
| 'Apt 53 (215 Int. 1112)', | ||
| 'international', | ||
| '{"country": "Colombia", "city": "Medellín", "apartment": "215 Int. 1112", "parking": "215 Int. 0226", "owner_entity": "ARIBIA LLC"}'::jsonb | ||
| ); |
There was a problem hiding this comment.
Non-idempotent INSERT may create duplicates on re-run.
Unlike the other property inserts, this INSERT lacks an ON CONFLICT clause. Since tax_pin is NULL for this international property, and PostgreSQL's UNIQUE constraint allows multiple NULL values, re-running this migration will insert duplicate Medellín rows.
Consider one of these approaches:
Option 1: Add a guard with NOT EXISTS
-- Medellín: International property held by ARIBIA LLC
-INSERT INTO cc_properties (property_name, address, unit, property_type, metadata)
-VALUES (
+INSERT INTO cc_properties (property_name, address, unit, property_type, metadata)
+SELECT
'Medellín',
'Urbanización Plaza De Colores, Carrera 76, Medellín, Colombia',
'Apt 53 (215 Int. 1112)',
'international',
'{"country": "Colombia", "city": "Medellín", "apartment": "215 Int. 1112", "parking": "215 Int. 0226", "owner_entity": "ARIBIA LLC"}'::jsonb
-);
+WHERE NOT EXISTS (
+ SELECT 1 FROM cc_properties WHERE property_name = 'Medellín'
+);Option 2: Add a unique constraint on property_name (if appropriate for the schema)
-- If property_name should be unique, add constraint first
ALTER TABLE cc_properties ADD CONSTRAINT uq_cc_properties_property_name UNIQUE (property_name);
-- Then use ON CONFLICT
INSERT INTO cc_properties (...)
VALUES (...)
ON CONFLICT (property_name) DO NOTHING;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| -- Medellín: International property held by ARIBIA LLC | |
| INSERT INTO cc_properties (property_name, address, unit, property_type, metadata) | |
| VALUES ( | |
| 'Medellín', | |
| 'Urbanización Plaza De Colores, Carrera 76, Medellín, Colombia', | |
| 'Apt 53 (215 Int. 1112)', | |
| 'international', | |
| '{"country": "Colombia", "city": "Medellín", "apartment": "215 Int. 1112", "parking": "215 Int. 0226", "owner_entity": "ARIBIA LLC"}'::jsonb | |
| ); | |
| -- Medellín: International property held by ARIBIA LLC | |
| INSERT INTO cc_properties (property_name, address, unit, property_type, metadata) | |
| SELECT | |
| 'Medellín', | |
| 'Urbanización Plaza De Colores, Carrera 76, Medellín, Colombia', | |
| 'Apt 53 (215 Int. 1112)', | |
| 'international', | |
| '{"country": "Colombia", "city": "Medellín", "apartment": "215 Int. 1112", "parking": "215 Int. 0226", "owner_entity": "ARIBIA LLC"}'::jsonb | |
| WHERE NOT EXISTS ( | |
| SELECT 1 FROM cc_properties WHERE property_name = 'Medellín' | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@migrations/0014_fix_properties.sql` around lines 66 - 74, The INSERT into
cc_properties for 'Medellín' is non-idempotent (tax_pin is NULL so unique
constraints won’t prevent duplicates); make it idempotent by either adding a NOT
EXISTS guard around the insert (check cc_properties for property_name =
'Medellín') or by adding a UNIQUE constraint on property_name and converting the
INSERT into an ON CONFLICT DO NOTHING using property_name as the conflict
target; update the migration to use one of these approaches so repeated runs do
not create duplicate Medellín rows.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Changes
Also updates related mortgage account and obligation metadata.
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit