chore: migrate soap api orgs to rest api#917
Conversation
📝 WalkthroughWalkthroughA new helper Poem
🚥 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. Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
Coverage Report
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Failure. Coverage is below 90%.Diff Coverage
|
1 similar comment
Failure. Coverage is below 90%.Diff Coverage
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/sage_intacct/helpers.py (1)
87-94:⚠️ Potential issue | 🟡 MinorDouble
sync_dimensionscall on first migration.When
validate_rest_api_connectionsuccessfully migrates a workspace, it callssync_dimensions(line 160). Thencheck_interval_and_sync_dimensioncallssync_dimensionsagain at line 92 (sincedestination_synced_atwill still satisfy the condition). This results in two full dimension syncs back-to-back on the first run after migration.Consider having
validate_rest_api_connectionreturn a boolean indicating whether a sync was performed, and skip the second sync here if so.
🤖 Fix all issues with AI agents
In `@apps/sage_intacct/helpers.py`:
- Around line 149-159: After updating the DB with
FeatureConfig.objects.filter(...).update(...), invalidate or update the cached
feature entry so subsequent calls to
FeatureConfig.get_feature_config(workspace_id=workspace_id,
key='migrated_to_rest_api') return True immediately; implement this by calling
the existing cache invalidation helper (e.g., FeatureConfig.invalidate_cache or
clear_cache) for that workspace/key or, if no helper exists, write a small
helper to set the cached value to True (or add a force_refresh flag to
get_feature_config and call it) right after the update so
get_sage_intacct_connection stops using the SOAP connector.
Coverage Report
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Failure. Coverage is below 90%.Diff Coverage
|
Coverage Report
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Diff Coverage
|
Coverage Report
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Diff Coverage
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/sage_intacct/helpers.py (1)
88-93:⚠️ Potential issue | 🟠 MajorBug:
sync_dimensionscalled twice on migration.When
validate_rest_api_connectionsucceeds for the first time, it callssync_dimensionsat line 164. Control then returns here, but the localworkspaceobject (fetched at line 86) is stale —destination_synced_atis stillNoneeven thoughsync_dimensionsjust updated it via a separate query. So line 92 evaluates toTrueandsync_dimensionsruns a second time.Either skip the outer sync when
validate_rest_api_connectionalready performed one (e.g., by returning a boolean), or remove thesync_dimensionscall from insidevalidate_rest_api_connectionand let the caller handle it.
No description provided.