feat(spp_change_request_v2): CR UX redesign with stage-based workflow#61
Open
feat(spp_change_request_v2): CR UX redesign with stage-based workflow#61
Conversation
Stage-based CR workflow, audit logging, document validation, flexible registrant, review comparison, revision wizard, conflict detection improvements, and UX fixes.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| changes = strategy.preview(rec) or {} | ||
| # Use sudo to bypass record rules (e.g. global disabled-registrant | ||
| # rules on spp.group.membership) — this is read-only preview logic. | ||
| sudo_rec = rec.sudo() |
Check warning
Code scanning / Semgrep OSS
Semgrep Finding: semgrep.odoo-sudo-without-context Warning
| changes = strategy.preview(sudo_self) or {} | ||
| except Exception as e: | ||
| _logger.warning("Error computing preview for CR ID %s: %s", self.id, e) | ||
| return ( |
Check warning
Code scanning / Semgrep OSS
Semgrep Finding: semgrep.odoo-sudo-without-context Warning
| @@ -720,27 +976,153 @@ def _generate_preview_html(self): | |||
| else: | |||
Check warning
Code scanning / Semgrep OSS
Semgrep Finding: semgrep.odoo-sudo-without-context Warning
| changes = strategy.preview(sudo_self) or {} | ||
| except Exception as e: | ||
| _logger.warning("Error computing review comparison for CR ID %s: %s", self.id, e) | ||
| return ( |
Check warning
Code scanning / Semgrep OSS
Semgrep Finding: semgrep.odoo-sudo-without-context Warning
| changes = strategy.preview(sudo_self) or {} | ||
| self.preview_json_snapshot = json.dumps(changes, indent=2, default=str) | ||
|
|
||
| def action_apply(self): |
Check warning
Code scanning / Semgrep OSS
Semgrep Finding: semgrep.odoo-sudo-without-context Warning
| strategy.apply(sudo_self) | ||
|
|
||
| def action_preview_changes(self): | ||
| """Preview what changes will be applied (returns data dict).""" |
Check warning
Code scanning / Semgrep OSS
Semgrep Finding: semgrep.odoo-sudo-without-context Warning
| "change_request_id": self.id, | ||
| "action": action, | ||
| "user_id": self.env.user.id, | ||
| "notes": notes, |
Check warning
Code scanning / Semgrep OSS
Semgrep Finding: semgrep.odoo-sudo-without-context Warning
| data["details"] = details | ||
|
|
||
| self.env["spp.event.data"].sudo().create( # nosemgrep: odoo-sudo-without-context | ||
| self.env["spp.event.data"].sudo().create( |
Check warning
Code scanning / Semgrep OSS
Semgrep Finding: semgrep.odoo-sudo-without-context Warning
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant redesign of the Change Request (CR) user experience, moving from a monolithic form to a structured, stage-based workflow. The changes aim to improve clarity, enforce data integrity, and enhance the overall usability of the CR system. Key improvements include a guided creation process, detailed activity logging, and more robust document and conflict management, ensuring a smoother and more controlled experience for users and validators. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 19.0 #61 +/- ##
==========================================
- Coverage 71.36% 64.66% -6.70%
==========================================
Files 319 274 -45
Lines 25878 21278 -4600
==========================================
- Hits 18467 13760 -4707
- Misses 7411 7518 +107
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a significant and well-executed UX redesign for the Change Request module, shifting to a stage-based workflow. The changes are comprehensive, including new models for audit logging, stage-specific forms, and various UI enhancements. The use of sudo() in read-only and post-approval operations is well-justified. The code is clean and demonstrates good practices, including forward-looking fixes for compatibility with upcoming Odoo versions. I've identified a couple of minor inconsistencies in the JavaScript code that could be addressed for improved maintainability.
| /** @odoo-module **/ | ||
|
|
||
| import {Component, useEffect, useRef} from "@odoo/owl"; | ||
| import {Component, useEffect, useRef, onWillUnmount} from "@odoo/owl"; |
There was a problem hiding this comment.
The onWillUnmount hook is imported but not used within the SearchDelayField component. It can be removed to keep the imports clean.
Suggested change
| import {Component, useEffect, useRef, onWillUnmount} from "@odoo/owl"; | |
| import {Component, useEffect, useRef} from "@odoo/owl"; |
Comment on lines
+59
to
+66
| this.fileViewer.open({ | ||
| isImage: Boolean(record.mimetype.startsWith("image/")), | ||
| isVideo: Boolean(record.mimetype.startsWith("video/")), | ||
| isViewable: true, | ||
| displayName: record.name, | ||
| defaultSource: fileUrl, | ||
| downloadUrl: fileUrl, | ||
| }); |
There was a problem hiding this comment.
For consistency with the change made in spp_dms/static/src/js/preview_binary_field.esm.js and the PR description, the downloadUrl property should probably be removed from the fileViewer.open() call. It seems this property might be deprecated or no longer necessary.
this.fileViewer.open({
isImage: Boolean(record.mimetype.startsWith("image/")),
isVideo: Boolean(record.mimetype.startsWith("video/")),
isViewable: true,
displayName: record.name,
defaultSource: fileUrl,
});
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.
Why is this change needed?
Syncs the latest CR UX redesign changes from openspp-modules-v2 (PR #299). The module has diverged significantly since the initial port (PR #29, merged Feb 11) with substantial improvements made after Feb 23.
How was the change implemented?
spp.change.request.logmodel tracking all state transitions_()variable naming fix, no_create/no_open on lookup fieldsAlso includes minor fix in
spp_dms(removedownloadUrlfrom file viewer).New unit tests
Updated existing tests to match new workflow (stage navigation, document validation)
Unit tests executed by the author
spp_change_request_v2tests pass (28 tests, 0 failures) in openspp-modules-v2How to test manually
spp_change_request_v2Related links
openspp-modules-v2 PR: OpenSPP/openspp-modules-v2#299