Skip to content

Import export study#3962

Open
ghazwarhili wants to merge 4 commits into
mainfrom
import-export-study
Open

Import export study#3962
ghazwarhili wants to merge 4 commits into
mainfrom
import-export-study

Conversation

@ghazwarhili

Copy link
Copy Markdown
Contributor

PR Summary

Import export study

@coderabbitai

coderabbitai Bot commented May 27, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR refactors the network modifications export from node-level to study-level. The fetchNetworkModificationsToExport function is relocated from network-modifications.ts to network.ts with a single-parameter signature, and the export menu is restricted to root nodes while the handler now exports study-wide data as exportStudy.json.

Changes

Study-level Network Modifications Export

Layer / File(s) Summary
Service layer: study-level export endpoint
src/services/study/network-modifications.ts, src/services/study/network.ts
fetchNetworkModificationsToExport is relocated from network-modifications.ts to network.ts and simplified from (studyUuid, nodeUuid) to (studyUuid). The function builds a network-modifications/export endpoint URL and returns the JSON response.
Export menu and handler: study-wide export
src/components/graph/menus/create-node-menu.tsx, src/components/network-modification-tree-pane.jsx
The export menu item changes from EXPORT_NODE to EXPORT_NODES with onRoot: true and added visibility requirement for NodeType.ROOT nodes. The handler removes its node parameter and now fetches and downloads study-level modifications as exportStudy.json using the relocated service function.

Suggested reviewers

  • Mathieu-Deharbe
  • Meklo
🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (1 warning, 2 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Import export study' is vague and does not clearly convey the actual changes, which involve refactoring export functionality to operate at study-level rather than node-level. Use a more descriptive title such as 'Refactor node export to study-level export' or 'Move export functionality from node-level to study-level' to clearly communicate the primary change.
Description check ❓ Inconclusive The description merely repeats the title without providing any meaningful detail about what changed or why, making it impossible to understand the purpose or scope of the changes. Expand the description to explain the refactoring: moving export logic from individual nodes to study-level, consolidating import locations, and updating the export behavior accordingly.
✅ Passed checks (2 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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


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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/components/network-modification-tree-pane.jsx`:
- Around line 329-331: The catch block in NetworkModificationTreePane (the
try/catch that currently does console.error('Error while exporting study:',
error)) must surface the failure to the user via the app's snackbar flow instead
of only logging; update that catch to call the existing snackbar function used
elsewhere in this component (e.g., enqueueSnackbar or showSnackbar /
showErrorSnackbar) with a clear message like "Failed to export study" and
include the error message/details, and keep the console.error for debugging.
Ensure you import or access the same snackbar instance/context used by other
handlers in this component and use it in the catch block where export errors are
currently logged.
🪄 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

Run ID: c0e4825e-02f3-4dce-899f-8a8f7adadbf7

📥 Commits

Reviewing files that changed from the base of the PR and between 9bbe621 and 530b14b.

📒 Files selected for processing (4)
  • src/components/graph/menus/create-node-menu.tsx
  • src/components/network-modification-tree-pane.jsx
  • src/services/study/network-modifications.ts
  • src/services/study/network.ts

Comment on lines 329 to 331
} catch (error) {
console.error('Error while exporting node Infos:', error);
console.error('Error while exporting study:', error);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Show a user-facing error when export fails.

Line 330 only logs to console, so export failures are silent in UI. Please surface the error through the existing snackbar flow.

Suggested patch
         } catch (error) {
-            console.error('Error while exporting study:', error);
+            snackWithFallback(snackError, error, { headerId: 'studyExportError' });
         }
📝 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
} catch (error) {
console.error('Error while exporting node Infos:', error);
console.error('Error while exporting study:', error);
}
} catch (error) {
snackWithFallback(snackError, error, { headerId: 'studyExportError' });
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/network-modification-tree-pane.jsx` around lines 329 - 331,
The catch block in NetworkModificationTreePane (the try/catch that currently
does console.error('Error while exporting study:', error)) must surface the
failure to the user via the app's snackbar flow instead of only logging; update
that catch to call the existing snackbar function used elsewhere in this
component (e.g., enqueueSnackbar or showSnackbar / showErrorSnackbar) with a
clear message like "Failed to export study" and include the error
message/details, and keep the console.error for debugging. Ensure you import or
access the same snackbar instance/context used by other handlers in this
component and use it in the catch block where export errors are currently
logged.

@sonarqubecloud

Copy link
Copy Markdown

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant