feat: enhance demo data loading #1574
Conversation
WalkthroughThis update modifies both the backend and frontend. In the backend, the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SideBar
participant Backend
User->>SideBar: Request demo domain import
SideBar->>SideBar: Set loading = true
SideBar->>Backend: Fetch demo domain data
Backend-->>SideBar: Respond with success or error
alt Success
SideBar->>SideBar: Trigger flash success message
else Failure
SideBar->>SideBar: Trigger flash error message
end
SideBar->>SideBar: Set loading = false
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
backend/core/views.py (2)
2493-2493: Simplify the static string assignment in import_dummy_domain.
The linedomain_name = f"DEMO"uses an f-string even though there are no placeholders. For clarity and to satisfy style guidelines, please remove the extraneous
fprefix (i.e. use"DEMO"instead).[minor]
🧰 Tools
🪛 Ruff (0.8.2)
2493-2493: f-string without any placeholders
Remove extraneous
fprefix(F541)
4476-4476: Ensure explicit import of Finding in FindingViewSet.status.
The code returnsreturn Response(dict(Finding.Status.choices))but static analysis indicates that
Findingmay be undefined or only available via a star import. For robustness and clarity, explicitly import theFindingmodel at the top of this file rather than relying on ambiguous star imports.🧰 Tools
🪛 Ruff (0.8.2)
4476-4476:
Findingmay be undefined, or defined from star imports(F405)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
backend/core/views.py(2 hunks)frontend/messages/en.json(2 hunks)frontend/messages/fr.json(2 hunks)frontend/src/lib/components/SideBar/SideBar.svelte(4 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
backend/core/views.py
2493-2493: f-string without any placeholders
Remove extraneous f prefix
(F541)
4476-4476: Finding may be undefined, or defined from star imports
(F405)
🪛 GitHub Actions: Backend Linters
backend/core/views.py
[error] 1-1: Ruff formatting check failed. 1 file would be reformatted. Please run 'ruff format' to fix code style issues in this file.
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: startup-functional-test (3.12)
- GitHub Check: migrations-check (3.12)
- GitHub Check: startup-docker-compose-test
- GitHub Check: build (3.12)
- GitHub Check: enterprise-functional-tests (3.12, chromium)
- GitHub Check: enterprise-startup-functional-test (3.12)
- GitHub Check: test (3.12)
- GitHub Check: functional-tests (3.12, chromium)
- GitHub Check: enterprise-startup-docker-compose-test
🔇 Additional comments (8)
frontend/messages/en.json (1)
1175-1175: New localization message added for demo data import status.This message will inform users when they attempt to import demo data that has already been imported, preventing redundant imports and improving user experience.
frontend/messages/fr.json (1)
1165-1165: French translation for demo data import status message.The French translation has been properly added for the demo data import message, maintaining consistent internationalization.
frontend/src/lib/components/SideBar/SideBar.svelte (6)
7-7: Added necessary imports for new functionality.The imports for the writable store and LoadingSpinner component are needed for the loading animation feature implementation.
Also applies to: 22-22
223-224: Added loading state management.A writable store is created to track loading state, which will be used to display the loading animation during demo data import.
226-226: Implementing loading state control.Loading state is set to true at the start of the import process and reset to false at all exit points (both success and error paths), ensuring the loading spinner is only displayed while the import is in progress.
Also applies to: 235-235, 247-247
229-233: Enhanced error handling for demo data import.The error handling now differentiates between a 500 status code (which indicates the demo data is already imported) and other errors, displaying an appropriate message in each case.
238-239: Improved success message handling.The success message is now sent to the flash store, providing consistent feedback to the user when the demo data is successfully imported.
288-294: Added loading spinner overlay during import.This conditional rendering block displays a loading spinner when the import process is active, providing visual feedback to the user and improving the overall user experience.
nas-tabchiche
changed the title
and avoid ugly error message in console
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
backend/core/views.py (1)
2493-2493: Remove unnecessary f-string prefixThe string "DEMO" doesn't contain any variables or expressions to interpolate, so the
fprefix is redundant.- domain_name = f"DEMO" + domain_name = "DEMO"🧰 Tools
🪛 Ruff (0.8.2)
2493-2493: f-string without any placeholders
Remove extraneous
fprefix(F541)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
backend/core/views.py(2 hunks)frontend/messages/en.json(1 hunks)frontend/messages/fr.json(1 hunks)frontend/src/lib/components/SideBar/SideBar.svelte(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/messages/en.json
- frontend/messages/fr.json
- frontend/src/lib/components/SideBar/SideBar.svelte
🧰 Additional context used
🪛 Ruff (0.8.2)
backend/core/views.py
2493-2493: f-string without any placeholders
Remove extraneous f prefix
(F541)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: startup-functional-test (3.12)
- GitHub Check: startup-docker-compose-test
- GitHub Check: enterprise-startup-functional-test (3.12)
- GitHub Check: enterprise-startup-docker-compose-test
- GitHub Check: build (3.12)
- GitHub Check: enterprise-functional-tests (3.12, chromium)
- GitHub Check: test (3.12)
- GitHub Check: functional-tests (3.12, chromium)
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
backend/core/views.py (3)
2493-2523: Solid implementation for preventing duplicate demo data importsThe change from dynamic timestamps to a static domain name ("DEMO") effectively enables the application to prevent multiple imports of the same demo data, as mentioned in the PR objectives. This is a good approach since it allows the frontend to easily check if the demo data already exists.
🧰 Tools
🪛 Ruff (0.8.2)
2493-2493: f-string without any placeholders
Remove extraneous
fprefix(F541)
2512-2512: Consider the impact of removing exception tracebacksRemoving
exc_info=Truefrom the logger call means that stack traces won't be included in the logs. While this makes logs cleaner, it might make troubleshooting more difficult if unexpected issues occur with JSON parsing.Is this change intentional? Consider whether this information might be useful for debugging in production environments.
2518-2518: Consider the impact of removing exception tracebacksSimilar to the previous comment, removing
exc_info=Truefrom this general exception logger will result in less detailed error information in the logs.Confirm that this is an intentional change to reduce log verbosity rather than an accidental omission.
The demo data domain is created always with the same name. If the domain already exists a popup will say that demo data is already existing.
Additionally, during loading of data, an animation is shown.
Summary by CodeRabbit
New Features
Bug Fixes