Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the possibility to draft a risk acceptance #1285

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

Axxiar
Copy link
Contributor

@Axxiar Axxiar commented Dec 30, 2024

Summary by CodeRabbit

Release Notes

  • New Features

    • Enhanced risk acceptance workflow with new submit and draft actions
    • Added more granular state management for risk acceptance process
  • Improvements

    • Updated localization messages for risk acceptance states
    • Improved form validation for approver field
    • Added more precise error handling in risk acceptance actions
  • Backend Changes

    • Updated linter configurations
    • Modified risk acceptance model and view logic
  • Frontend Changes

    • Updated component logic for risk acceptance forms
    • Refined user interface messaging for risk acceptance states

This is not finished yet, some little things remain to be done.
@Axxiar Axxiar marked this pull request as draft December 30, 2024 19:58
@Axxiar Axxiar requested a review from Mohamed-Hacene January 20, 2025 12:19
@Axxiar Axxiar marked this pull request as ready for review January 20, 2025 18:30
@Axxiar
Copy link
Contributor Author

Axxiar commented Jan 20, 2025

I reopen this to be reviewed, there still are the issues with CodeFactor @Mohamed-Hacene

@nas-tabchiche
Copy link
Contributor

@coderabbitai full review

Copy link

coderabbitai bot commented Jan 22, 2025

✅ Actions performed

Full review triggered.

Copy link

coderabbitai bot commented Jan 22, 2025

Walkthrough

This pull request introduces comprehensive changes across multiple files to enhance the risk acceptance workflow. The modifications span from backend logic in Django views and models to frontend components and localization files. Key updates include new actions for submitting and drafting risk acceptances, changes to state management, updates to translation keys, and adjustments to form validation to allow optional approvers. The changes aim to provide more flexible and precise control over the risk acceptance process.

Changes

File Change Summary
.github/workflows/backend-linters.yaml Updated ruff linter version from 0.9.1 to 0.9.2
.pre-commit-config.yaml Updated ruff-pre-commit repository revision from v0.4.1 to v0.9.2
backend/core/models.py Replaced translatable strings with plain string literals in ACCEPTANCE_STATE choices
backend/core/views.py Added submit and draft actions, enhanced error handling in state change methods
frontend/messages/* Updated translation keys from riskAcceptanceReviewMessage to riskAcceptanceValidatingReviewMessage across multiple language files
frontend/src/lib/components/DetailView/DetailView.svelte Added new popup settings, modified edit button logic, introduced submit button
frontend/src/lib/components/Forms/ModelForm/RiskAcceptanceForm.svelte Added nullable property to approver field
frontend/src/lib/utils/schemas.ts Modified RiskAcceptanceSchema to make approver optional and nullable
frontend/src/routes/(app)/(internal)/[model=urlmodel]/[id=uuid]/+page.server.ts Added submit and draft server-side actions

Sequence Diagram

sequenceDiagram
    participant User
    participant Frontend
    participant Backend
    participant Approver

    User->>Frontend: Create Risk Acceptance
    Frontend->>Backend: Save as Draft
    Backend-->>Frontend: Draft Saved

    User->>Frontend: Submit Risk Acceptance
    alt Approver Exists
        Frontend->>Backend: Submit for Validation
        Backend->>Approver: Notify for Review
        Approver->>Backend: Review Risk Acceptance
        Backend-->>Frontend: Update Status
    else No Approver
        Frontend-->>User: Show Error
    end
Loading

Poem

🐰 Risk acceptance, a dance of state,
Drafting, submitting, we navigate
Approvers welcome, optional too
Workflows bloom like carrots anew
Code hops forward with graceful might! 🥕

✨ Finishing Touches
  • 📝 Generate Docstrings (Beta)

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🔭 Outside diff range comments (2)
backend/core/models.py (2)

Line range hint 3418-3486: Add validation for expiry date.

The expiry_date field is critical for tracking the validity of risk acceptances, but it's currently nullable. Consider adding validation to ensure an expiry date is always set when transitioning to the "accepted" state.

Add validation in the set_state method:

     def set_state(self, state):
         self.state = state
         if state == "accepted":
+            if not self.expiry_date:
+                raise ValidationError(_("Expiry date is required for risk acceptance"))
             self.accepted_at = datetime.now()
             # iterate over the risk scenarios to set their treatment to accepted
             for scenario in self.risk_scenarios.all():

Line range hint 3469-3486: Add state transition validation.

The set_state method allows any state transition without validation. This could lead to invalid state changes like transitioning from "rejected" to "submitted".

Add state transition validation:

+    VALID_TRANSITIONS = {
+        "created": ["submitted"],
+        "submitted": ["accepted", "rejected"],
+        "accepted": ["revoked"],
+        "rejected": ["created"],
+        "revoked": ["created"],
+    }
+
     def set_state(self, state):
+        if state not in self.VALID_TRANSITIONS.get(self.state, []):
+            raise ValidationError(
+                _("Invalid state transition from %(from_state)s to %(to_state)s") % 
+                {"from_state": self.state, "to_state": state}
+            )
         self.state = state
         if state == "accepted":
             self.accepted_at = datetime.now()
🧹 Nitpick comments (4)
frontend/src/routes/(app)/(internal)/[model=urlmodel]/[id=uuid]/+page.server.ts (1)

179-184: Consider using different success messages for submit and draft actions.

Both actions currently use m.successfullyValidatedObject(). Consider using distinct messages to better reflect the specific action taken.

Also applies to: 214-219

frontend/messages/it.json (1)

456-456: Fix typo in Italian translation.

There's a typo in the message: "rotnare" should be "tornare" (in "non puoi rotnare indietro").

-  "riskAcceptanceValidatedMessage": "L'accettazione di questo rischio è attualmente confermata. Puoi revocarla in qualsiasi momento, ma non puoi rotnare indietro. Si consiglia di duplicarla con una versione diversa se necessario.",
+  "riskAcceptanceValidatedMessage": "L'accettazione di questo rischio è attualmente confermata. Puoi revocarla in qualsiasi momento, ma non puoi tornare indietro. Si consiglia di duplicarla con una versione diversa se necessario.",
backend/core/models.py (1)

Line range hint 3459-3462: Follow Django's permission naming convention.

The custom permission name doesn't follow Django's convention of <verb>_<model>. Also, the permission description could be more specific about the action.

Update the permission definition:

     class Meta:
         permissions = [
-            ("approve_riskacceptance", "Can validate/rejected risk acceptances")
+            ("approve_risk_acceptance", "Can approve or reject risk acceptance requests")
         ]
.github/workflows/backend-linters.yaml (1)

Line range hint 39-42: Consider enabling ruff linter checks.

The ruff linter check is commented out with a note about waiting for codebase cleanup. Consider creating a tracking issue for this cleanup to ensure it's not forgotten.

Would you like me to help create an issue to track the codebase cleanup task?

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3dc15d9 and f3f9eb6.

📒 Files selected for processing (22)
  • .github/workflows/backend-linters.yaml (1 hunks)
  • .pre-commit-config.yaml (1 hunks)
  • backend/core/models.py (1 hunks)
  • backend/core/views.py (2 hunks)
  • frontend/messages/ar.json (1 hunks)
  • frontend/messages/cs.json (1 hunks)
  • frontend/messages/de.json (1 hunks)
  • frontend/messages/en.json (2 hunks)
  • frontend/messages/es.json (1 hunks)
  • frontend/messages/fr.json (2 hunks)
  • frontend/messages/hi.json (1 hunks)
  • frontend/messages/it.json (1 hunks)
  • frontend/messages/nl.json (1 hunks)
  • frontend/messages/pl.json (1 hunks)
  • frontend/messages/pt.json (1 hunks)
  • frontend/messages/ro.json (1 hunks)
  • frontend/messages/sv.json (1 hunks)
  • frontend/messages/ur.json (1 hunks)
  • frontend/src/lib/components/DetailView/DetailView.svelte (6 hunks)
  • frontend/src/lib/components/Forms/ModelForm/RiskAcceptanceForm.svelte (1 hunks)
  • frontend/src/lib/utils/schemas.ts (1 hunks)
  • frontend/src/routes/(app)/(internal)/[model=urlmodel]/[id=uuid]/+page.server.ts (1 hunks)
🔇 Additional comments (23)
frontend/src/lib/components/Forms/ModelForm/RiskAcceptanceForm.svelte (1)

54-54: LGTM! The nullable property aligns with schema changes.

The addition of nullable={true} correctly enables the form to handle null values for the approver field, which is consistent with the updated schema definition.

frontend/src/lib/components/DetailView/DetailView.svelte (2)

33-37: LGTM! Well-structured popup configuration.

The popup settings are clearly defined with appropriate placement and event trigger.


382-419: Great user feedback implementation!

The implementation provides excellent user feedback by:

  • Disabling buttons when appropriate (no approver)
  • Using popups to explain why buttons are disabled
  • Clear visual indicators with icons
frontend/src/lib/utils/schemas.ts (1)

168-168: LGTM! Schema change properly supports the draft functionality.

Making the approver field optional and nullable correctly enables the risk acceptance draft feature while maintaining type safety.

backend/core/views.py (2)

1657-1666: LGTM! The submit action implementation looks good.

The method correctly validates the presence of an approver before allowing submission and returns appropriate error responses.


1713-1725: LGTM! The perform_update validation looks good.

The method correctly validates the approver's permissions on the risk acceptance's project folder.

frontend/messages/ar.json (1)

457-457: LGTM! The Arabic translation for risk acceptance validation message is accurate.

The translation key riskAcceptanceValidatingReviewMessage correctly conveys the meaning of the risk acceptance validation state.

frontend/messages/ur.json (1)

456-456: LGTM! The key name change better reflects the validation state.

The new key "riskAcceptanceValidatingReviewMessage" more precisely describes the validation state in the risk acceptance workflow, and the message content appropriately warns users about the irreversible nature of their decisions.

frontend/messages/cs.json (1)

454-454: LGTM! The key name change and translation are consistent.

The new key "riskAcceptanceValidatingReviewMessage" matches the change in other localization files, and the Czech translation maintains semantic consistency with the risk acceptance workflow.

frontend/messages/hi.json (1)

456-456: LGTM! Key renamed for better clarity.

The key has been appropriately renamed from "riskAcceptanceReviewMessage" to "riskAcceptanceValidatingReviewMessage" to better reflect its specific purpose in the validation workflow, while preserving the existing translation.

frontend/messages/sv.json (1)

456-456: LGTM! Consistent key renaming across localization files.

The key has been consistently renamed from "riskAcceptanceReviewMessage" to "riskAcceptanceValidatingReviewMessage" to match the changes in other localization files, while preserving the existing translation.

frontend/messages/pl.json (1)

456-456: LGTM! Good terminology standardization.

The key rename from riskAcceptanceReviewMessage to riskAcceptanceValidatingReviewMessage better reflects the state of the risk acceptance process while maintaining consistent messaging across all supported languages.

frontend/messages/ro.json (1)

456-456: LGTM! Consistent with other language files.

The key update matches the standardization applied across other language files while preserving the Romanian translation.

frontend/messages/pt.json (1)

456-456: LGTM! Maintains translation consistency.

The key change aligns with the standardization across other language files while keeping the Portuguese translation intact.

frontend/messages/nl.json (1)

456-456: LGTM! Clear and accurate Dutch translation.

The translation effectively communicates the validation process and irreversibility warning to Dutch users.

frontend/messages/es.json (1)

456-456: LGTM! Clear and accurate Spanish translation.

The translation effectively communicates the validation process and irreversibility warning to Spanish users.

frontend/messages/de.json (1)

456-456: LGTM! German translation for risk acceptance validation message.

The translation is accurate and consistent with the risk acceptance workflow.

frontend/messages/en.json (1)

149-149: LGTM! Complete set of English messages for the risk acceptance workflow.

The new messages provide clear guidance through the risk acceptance process:

  1. Checking for approver before submission
  2. Informing about the submission to approver
  3. Guiding the approver during validation

This improves the user experience by making the workflow steps clearer.

Also applies to: 460-462

frontend/messages/fr.json (1)

460-460: LGTM! French translation for risk acceptance validation message.

The translation is accurate and consistent with the risk acceptance workflow.

.pre-commit-config.yaml (2)

9-9: Verify ruff version compatibility and breaking changes.

The version has been updated from v0.4.1 to v0.9.2, which is a significant version jump.

Let's verify the version and check for any breaking changes:

✅ Verification successful

yaml/,/```/'


---

yaml/,/```/' 

# Get release notes from main Ruff repository
curl -s https://api.github.com/repos/astral-sh/ruff/releases/tags/v0.9.2 | jq -r '.body'

Version update to v0.9.2 is appropriate

The version has been verified to exist and is actually the latest release (January 16, 2025) of ruff-pre-commit.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify ruff version and check changelog for breaking changes

# Check if the version exists
curl -s https://api.github.com/repos/astral-sh/ruff-pre-commit/tags | jq -r '.[].name' | grep -q "v0.9.2" && echo "Version exists" || echo "Version not found"

# Get changelog entries between v0.4.1 and v0.9.2
curl -s https://raw.githubusercontent.com/astral-sh/ruff/main/CHANGELOG.md | awk '/## \[0\.9\.2\]/,/## \[0\.4\.1\]/'

Length of output: 263


Script:

#!/bin/bash
# Check latest version and its release date
curl -s https://api.github.com/repos/astral-sh/ruff-pre-commit/releases/latest | jq -r '"\(.tag_name) released on \(.published_at)"'

# Get release notes for v0.9.2
curl -s https://api.github.com/repos/astral-sh/ruff-pre-commit/releases/tags/v0.9.2 | jq -r '.body'

# Check sample config from the repo
curl -s https://raw.githubusercontent.com/astral-sh/ruff-pre-commit/main/README.md | awk '/

Length of output: 449


Script:

#!/bin/bash
# Get example configuration from README
curl -s https://raw.githubusercontent.com/astral-sh/ruff-pre-commit/main/README.md | awk '/

Length of output: 116


Line range hint 11-14: Consider enabling the ruff linter.

The ruff linter hook is commented out. Since we're running the formatter, it would be beneficial to also run the linter to catch potential issues early in development.

Let's check if there are any existing linting issues that might be blocking this:

✅ Verification successful

Enabling the ruff linter is safe and recommended

The project has a complete ruff configuration with well-selected rules, and the formatter is already working successfully. There are no technical blockers or conflicting tools that would prevent enabling the linter.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for linting issues in the backend code

# Find and analyze Python files in the backend directory
fd -e py . backend/ --exec ruff check {} || true

Length of output: 7647


Script:

#!/bin/bash
# Check for ruff configuration and project dependencies
echo "=== Checking for ruff configuration files ==="
fd -e toml -e ini -H ruff
fd pyproject.toml

echo -e "\n=== Checking Python dependencies ==="
fd "requirements.*\.txt$|pyproject\.toml$|setup\.py$" -x cat {}

echo -e "\n=== Checking for other linting tools ==="
fd "\.flake8$|\.pylintrc$|setup\.cfg$" -x cat {}

Length of output: 2034


Script:

#!/bin/bash
# Check ruff configuration
echo "=== Checking ruff configuration ==="
cat ruff.toml

echo -e "\n=== Checking pre-commit config ==="
cat .pre-commit-config.yaml

Length of output: 1110

.github/workflows/backend-linters.yaml (2)

34-34: LGTM! Version matches pre-commit config.

The ruff version (0.9.2) is consistent with the version specified in .pre-commit-config.yaml.


Line range hint 8-11: Verify Python and Ubuntu versions compatibility.

The workflow uses Python 3.12 and Ubuntu 24.04, which are relatively new versions.

Let's verify the stability and availability of these versions:

Comment on lines +153 to +221
submit: async ({ request, fetch, params }) => {
const formData = await request.formData();
const schema = z.object({ urlmodel: z.string(), id: z.string().uuid() });
const submitForm = await superValidate(formData, zod(schema));

const urlmodel = submitForm.data.urlmodel;
const id = submitForm.data.id;
const endpoint = `${BASE_API_URL}/${urlmodel}/${id}/submit/`;

if (!submitForm.valid) {
return fail(400, { form: submitForm });
}

const requestInitOptions: RequestInit = {
method: 'POST'
};
const res = await fetch(endpoint, requestInitOptions);
if (!res.ok) {
const response = await res.json();
if (response.non_field_errors) {
setError(submitForm, 'non_field_errors', response.non_field_errors);
}
return fail(400, { form: submitForm });
}
const model: string = urlParamModelVerboseName(params.model!);
// TODO: reference object by name instead of id
return message(
submitForm,
m.successfullyValidatedObject({
object: safeTranslate(model).toLowerCase(),
id: id
})
);
},

draft: async ({ request, fetch, params }) => {
const formData = await request.formData();
const schema = z.object({ urlmodel: z.string(), id: z.string().uuid() });
const draftForm = await superValidate(formData, zod(schema));

const urlmodel = draftForm.data.urlmodel;
const id = draftForm.data.id;
const endpoint = `${BASE_API_URL}/${urlmodel}/${id}/draft/`;

if (!draftForm.valid) {
return fail(400, { form: draftForm });
}

const requestInitOptions: RequestInit = {
method: 'POST'
};
const res = await fetch(endpoint, requestInitOptions);
if (!res.ok) {
const response = await res.json();
if (response.non_field_errors) {
setError(draftForm, 'non_field_errors', response.non_field_errors);
}
return fail(400, { form: draftForm });
}
const model: string = urlParamModelVerboseName(params.model!);
// TODO: reference object by name instead of id
return message(
draftForm,
m.successfullyValidatedObject({
object: safeTranslate(model).toLowerCase(),
id: id
})
);
},
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider extracting common action handling logic.

The submit and draft actions contain significant code duplication. Consider extracting the common pattern into a reusable function.

Here's a suggested refactor:

+const handleModelAction = async ({ request, fetch, params, action }) => {
+  const formData = await request.formData();
+  const schema = z.object({ urlmodel: z.string(), id: z.string().uuid() });
+  const form = await superValidate(formData, zod(schema));
+
+  const urlmodel = form.data.urlmodel;
+  const id = form.data.id;
+  const endpoint = `${BASE_API_URL}/${urlmodel}/${id}/${action}/`;
+
+  if (!form.valid) {
+    return fail(400, { form });
+  }
+
+  const requestInitOptions: RequestInit = {
+    method: 'POST'
+  };
+  const res = await fetch(endpoint, requestInitOptions);
+  if (!res.ok) {
+    const response = await res.json();
+    if (response.non_field_errors) {
+      setError(form, 'non_field_errors', response.non_field_errors);
+    }
+    return fail(400, { form });
+  }
+  const model: string = urlParamModelVerboseName(params.model!);
+  return message(
+    form,
+    m.successfullyValidatedObject({
+      object: safeTranslate(model).toLowerCase(),
+      id: id
+    })
+  );
+};

 export const actions: Actions = {
-  submit: async ({ request, fetch, params }) => {
-    const formData = await request.formData();
-    const schema = z.object({ urlmodel: z.string(), id: z.string().uuid() });
-    const submitForm = await superValidate(formData, zod(schema));
-
-    const urlmodel = submitForm.data.urlmodel;
-    const id = submitForm.data.id;
-    const endpoint = `${BASE_API_URL}/${urlmodel}/${id}/submit/`;
-
-    if (!submitForm.valid) {
-      return fail(400, { form: submitForm });
-    }
-
-    const requestInitOptions: RequestInit = {
-      method: 'POST'
-    };
-    const res = await fetch(endpoint, requestInitOptions);
-    if (!res.ok) {
-      const response = await res.json();
-      if (response.non_field_errors) {
-        setError(submitForm, 'non_field_errors', response.non_field_errors);
-      }
-      return fail(400, { form: submitForm });
-    }
-    const model: string = urlParamModelVerboseName(params.model!);
-    return message(
-      submitForm,
-      m.successfullyValidatedObject({
-        object: safeTranslate(model).toLowerCase(),
-        id: id
-      })
-    );
-  },
+  submit: async (event) => handleModelAction({ ...event, action: 'submit' }),
+  draft: async (event) => handleModelAction({ ...event, action: 'draft' }),
-  draft: async ({ request, fetch, params }) => {
-    const formData = await request.formData();
-    const schema = z.object({ urlmodel: z.string(), id: z.string().uuid() });
-    const draftForm = await superValidate(formData, zod(schema));
-
-    const urlmodel = draftForm.data.urlmodel;
-    const id = draftForm.data.id;
-    const endpoint = `${BASE_API_URL}/${urlmodel}/${id}/draft/`;
-
-    if (!draftForm.valid) {
-      return fail(400, { form: draftForm });
-    }
-
-    const requestInitOptions: RequestInit = {
-      method: 'POST'
-    };
-    const res = await fetch(endpoint, requestInitOptions);
-    if (!res.ok) {
-      const response = await res.json();
-      if (response.non_field_errors) {
-        setError(draftForm, 'non_field_errors', response.non_field_errors);
-      }
-      return fail(400, { form: draftForm });
-    }
-    const model: string = urlParamModelVerboseName(params.model!);
-    return message(
-      draftForm,
-      m.successfullyValidatedObject({
-        object: safeTranslate(model).toLowerCase(),
-        id: id
-      })
-    );
-  },
📝 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
submit: async ({ request, fetch, params }) => {
const formData = await request.formData();
const schema = z.object({ urlmodel: z.string(), id: z.string().uuid() });
const submitForm = await superValidate(formData, zod(schema));
const urlmodel = submitForm.data.urlmodel;
const id = submitForm.data.id;
const endpoint = `${BASE_API_URL}/${urlmodel}/${id}/submit/`;
if (!submitForm.valid) {
return fail(400, { form: submitForm });
}
const requestInitOptions: RequestInit = {
method: 'POST'
};
const res = await fetch(endpoint, requestInitOptions);
if (!res.ok) {
const response = await res.json();
if (response.non_field_errors) {
setError(submitForm, 'non_field_errors', response.non_field_errors);
}
return fail(400, { form: submitForm });
}
const model: string = urlParamModelVerboseName(params.model!);
// TODO: reference object by name instead of id
return message(
submitForm,
m.successfullyValidatedObject({
object: safeTranslate(model).toLowerCase(),
id: id
})
);
},
draft: async ({ request, fetch, params }) => {
const formData = await request.formData();
const schema = z.object({ urlmodel: z.string(), id: z.string().uuid() });
const draftForm = await superValidate(formData, zod(schema));
const urlmodel = draftForm.data.urlmodel;
const id = draftForm.data.id;
const endpoint = `${BASE_API_URL}/${urlmodel}/${id}/draft/`;
if (!draftForm.valid) {
return fail(400, { form: draftForm });
}
const requestInitOptions: RequestInit = {
method: 'POST'
};
const res = await fetch(endpoint, requestInitOptions);
if (!res.ok) {
const response = await res.json();
if (response.non_field_errors) {
setError(draftForm, 'non_field_errors', response.non_field_errors);
}
return fail(400, { form: draftForm });
}
const model: string = urlParamModelVerboseName(params.model!);
// TODO: reference object by name instead of id
return message(
draftForm,
m.successfullyValidatedObject({
object: safeTranslate(model).toLowerCase(),
id: id
})
);
},
const handleModelAction = async ({ request, fetch, params, action }) => {
const formData = await request.formData();
const schema = z.object({ urlmodel: z.string(), id: z.string().uuid() });
const form = await superValidate(formData, zod(schema));
const urlmodel = form.data.urlmodel;
const id = form.data.id;
const endpoint = `${BASE_API_URL}/${urlmodel}/${id}/${action}/`;
if (!form.valid) {
return fail(400, { form });
}
const requestInitOptions: RequestInit = {
method: 'POST'
};
const res = await fetch(endpoint, requestInitOptions);
if (!res.ok) {
const response = await res.json();
if (response.non_field_errors) {
setError(form, 'non_field_errors', response.non_field_errors);
}
return fail(400, { form });
}
const model: string = urlParamModelVerboseName(params.model!);
return message(
form,
m.successfullyValidatedObject({
object: safeTranslate(model).toLowerCase(),
id: id
})
);
};
export const actions: Actions = {
submit: async (event) => handleModelAction({ ...event, action: 'submit' }),
draft: async (event) => handleModelAction({ ...event, action: 'draft' }),
};

Comment on lines +1667 to +1672
# This set back risk acceptance to "Created"
@action(detail=True, methods=["post"], name="Draft risk acceptance")
def draft(self, request, pk):
self.get_object().set_state("created")
return Response({"results": "state updated to submitted"})

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix the success message in the draft action.

The success message incorrectly states "state updated to submitted" when it should be "state updated to created".

Apply this diff to fix the message:

-        return Response({"results": "state updated to submitted"})
+        return Response({"results": "state updated to created"})
📝 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
# This set back risk acceptance to "Created"
@action(detail=True, methods=["post"], name="Draft risk acceptance")
def draft(self, request, pk):
self.get_object().set_state("created")
return Response({"results": "state updated to submitted"})
# This set back risk acceptance to "Created"
@action(detail=True, methods=["post"], name="Draft risk acceptance")
def draft(self, request, pk):
self.get_object().set_state("created")
return Response({"results": "state updated to created"})

@@ -146,6 +146,7 @@
"acceptedAt": "Accepté le",
"rejectedAt": "Rejeté le",
"revokedAt": "Révoqué le",
"created": "Créé",
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add missing translations for new English messages.

The following messages are present in English but missing in German and French:

  • "created": "Created"
  • "riskAcceptanceSubmittingReviewMessage": "This will be sent to the approver for validation."
  • "riskAcceptanceMissingApproverMessage": "Approver required before submitting."

Please add the corresponding translations to maintain consistency across all language files.

Also applies to: 460-462

Comment on lines +3413 to +3417
("created", "Created"),
("submitted", "Submitted"),
("accepted", "Accepted"),
("rejected", "Rejected"),
("revoked", "Revoked"),
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Restore internationalization support for acceptance states.

The changes remove translation support for acceptance state choices by replacing gettext_lazy wrapped strings with plain strings. This will prevent the UI from displaying these states in the user's selected language.

Apply this diff to restore internationalization:

-        ("created", "Created"),
-        ("submitted", "Submitted"),
-        ("accepted", "Accepted"),
-        ("rejected", "Rejected"),
-        ("revoked", "Revoked"),
+        ("created", _("Created")),
+        ("submitted", _("Submitted")),
+        ("accepted", _("Accepted")),
+        ("rejected", _("Rejected")),
+        ("revoked", _("Revoked")),

Committable suggestion skipped: line range outside the PR's diff.

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.

2 participants