feat: user submission management v1#10
Conversation
|
Warning Rate limit exceeded@harshtandiya has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 12 minutes and 9 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR extends the forms_pro application with submission status tracking, user submission management, and form publication state handling. It adds backend API enhancements for linked doctypes and submission responses, introduces status fields to forms via FormGenerator, and implements frontend components and stores for viewing, editing, and managing form submissions with draft/submitted states. Changes
Sequence DiagramssequenceDiagram
participant User
participant Frontend as Frontend Store
participant API as Backend API
participant DB as Database
User->>Frontend: Load form submission page
Frontend->>API: GET /api/form/{form_id}
API->>DB: Fetch form (with linked_doctype)
API-->>Frontend: Return form + linked_doctype
Frontend->>API: GET /api/user/submissions/{form_id}
API->>DB: Query submissions by current user & doctype
API-->>Frontend: Return UserSubmissionResponse list
Frontend->>User: Render form + previous submissions
alt User edits existing submission
User->>Frontend: Click "View" on previous submission
Frontend->>API: GET /api/submission/{doctype}/{name}
API->>DB: Fetch submission document
API->>DB: Check read permissions
API-->>Frontend: Return submission dict
Frontend->>User: Load submission in edit form
User->>Frontend: Modify fields + Submit
Frontend->>API: POST /api/submission (submission_status=SUBMITTED)
API->>DB: Update submission + set status
API->>DB: Share document with owner
API-->>Frontend: Return submission name
else User creates new submission
User->>Frontend: Fill form fields
User->>Frontend: Click "Submit" or "Save as Draft"
Frontend->>API: POST /api/submission (submission_status=DRAFT/SUBMITTED)
API->>DB: Create submission in linked_doctype
API->>DB: Set fp_linked_form + fp_submission_status
API->>DB: Share with owner (read/write)
API-->>Frontend: Return submission name
Frontend->>API: GET /api/user/submissions/{form_id}
API-->>Frontend: Return updated submissions list
end
Frontend->>User: Show success or update submissions list
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Fix all issues with AI agents
In @forms_pro/api/submission.py:
- Around line 104-109: The current frappe.get_all call for variable submissions
queries linked_doctype by owner only and can return submissions from other
forms; update the filters in the frappe.get_all call (the get_all invocation
that uses linked_doctype and fields
["name","creation","modified","fp_submission_status"]) to include fp_linked_form
equal to the requested form_id so only submissions for that specific form are
returned.
In @forms_pro/utils/form_generator.py:
- Around line 67-73: The LINKED_FORM_FIELDOPTIONS dict is missing the
"mandatory" key referenced by tests; update LINKED_FORM_FIELDOPTIONS (the
constant defined in form_generator.py) to include a "mandatory" entry (e.g.,
"mandatory": 0) using the same type convention used elsewhere (int or bool) so
linked_form_field.mandatory matches LINKED_FORM_FIELDOPTIONS["mandatory"] in the
tests.
In @forms_pro/utils/test_form_generator.py:
- Around line 321-342: Change the next(...) call that finds linked_form_field to
use a safe default (next(..., None)) and then assert linked_form_field is not
None (reference: linked_form_field and form_generator.doctype.fields) to avoid
StopIteration; and replace direct dict indexing of LINKED_FORM_FIELDOPTIONS for
keys like "in_list_view" and "mandatory" with .get(...) (e.g.,
LINKED_FORM_FIELDOPTIONS.get("in_list_view")) or provide explicit defaults
before asserting equality against custom_field (references:
LINKED_FORM_FIELDOPTIONS and custom_field comparisons) so missing keys no longer
raise KeyError.
- Around line 250-262: The test is referencing missing keys in
LINKED_FORM_FIELDOPTIONS and uses next() without a default which can raise
StopIteration; update the test to either remove assertions for in_list_view and
mandatory or add those keys to LINKED_FORM_FIELDOPTIONS to match what's
expected, and change the lookup to use next((field for field in
form_generator.doctype.fields if field.fieldname ==
LINKED_FORM_FIELDOPTIONS["fieldname"]), None) then assert linked_form_field is
not None with a clear failure message before checking its attributes; keep
references to LINKED_FORM_FIELDOPTIONS, form_generator.doctype.fields and
linked_form_field when making the changes.
In @frontend/src/components/submission/FormUnpublishedState.vue:
- Line 8: Update the heading text in the FormUnpublishedState.vue component to
use the correct spelling "unpublished" (no hyphen); locate the <h1> element
currently rendering "Form is un-published!" and change it to "Form is
unpublished!" to ensure consistency across the UI.
In @frontend/src/pages/submission/PublicEdit.vue:
- Around line 161-174: The Update & Submit flow calls
editSubmissionStore.updateAndSubmitForm directly and bypasses form validation;
before invoking updateAndSubmitForm(pass submissionFormStore.fields), call the
form validation method (submissionFormStore.validate() or the existing
validateForm equivalent) and only proceed when it returns success; keep the
disabled/:loading/:tooltip bindings (isDirty, submissionFormStore.isLoading)
intact and ensure the validation check mirrors the standalone Submit button's
behavior so invalid data cannot be submitted.
In @frontend/src/router.ts:
- Around line 45-49: The "Public Edit Submission Page" route (name "Public Edit
Submission Page", path "/p/:route(.*)/edit/:submissionName") is missing an
authentication guard; add a beforeEnter guard like the one used on the "Form
Submission Page" route to block unauthenticated access. Locate the route
definition for the PublicEdit.vue component and add the same beforeEnter handler
(or call to your existing auth check function, e.g., requireAuth or isLoggedIn)
so it verifies login and redirects/blocks when the user is not authenticated.
In @frontend/src/stores/editSubmission.ts:
- Around line 11-16: The computed status checks use loose equality; update the
two computed properties isDraft and isSubmitted to use strict equality (replace
== with ===) when comparing submission.value?.fp_submission_status to "Draft"
and "Submitted" respectively so the comparisons use === and avoid type coercion.
- Around line 59-62: The issue: updateAndSubmitForm calls updateForm(data) and
submitForm() synchronously causing a race; fix by making updateAndSubmitForm
async and waiting for the field update to finish before submitting: ensure
updateForm(data) returns a Promise (or wrap it with Promise.resolve) and await
its completion (and optionally handle errors with try/catch) before calling
submitForm(); reference the functions updateAndSubmitForm, updateForm, and
submitForm so you can locate and modify the sequencing.
- Around line 20-30: Replace the manual isLoading ref with a computed getter
that returns submissionResource.value?.loading || false, remove the explicit
isLoading.value = true/false assignments inside initialize, and ensure
initialize only sets submissionDoctype, submissionName, and submissionResource
via createDocumentResource; update any references to isLoading to use the new
computed isLoading and rely on submissionResource.loading to reflect async fetch
state (symbols: initialize, isLoading, submissionResource,
createDocumentResource, PublicEdit.vue references).
In @frontend/src/stores/submissionForm.ts:
- Around line 28-42: The computed getters currentFormId and formIsPublished are
checking the ref itself and directly accessing nested properties, which can
throw when formResource.value or formResource.value.data is null; update both to
first guard against formResource.value and formResource.value.data (e.g., check
if formResource?.value and formResource.value.data exist or use optional
chaining) and return null when missing, then safely return
formResource.value.data.name for currentFormId and
formResource.value.data.is_published for formIsPublished.
- Around line 9-13: The UserSubmission type is missing the submission_status
field; update the exported type UserSubmission to include submission_status
(e.g., submission_status: string or the appropriate enum/union used by the
backend) so the frontend shape matches the backend UserSubmissionResponse;
change any consumers of UserSubmission to handle the new field where necessary
(look for references to UserSubmission in submissionForm.ts and related
selectors/components).
🧹 Nitpick comments (9)
frontend/src/components/FormBuilderHeader.vue (1)
70-83: Improved UX with clear published/unpublished indicators.The change successfully adds distinct visual feedback for published vs. unpublished states, which improves user clarity.
♻️ Optional simplification: use `v-else` instead of `v-else-if="!editFormStore.isPublished"`
Since the condition on line 78 is guaranteed to be true when reached (already ruled out
isUnsavedandisPublished), you can simplify tov-else:<CloudCheck class="w-4 h-4 text-gray-500" /> </Tooltip> <Tooltip - v-else-if="!editFormStore.isPublished" + v-else text="Form is not published" placement="bottom" >This is more idiomatic and slightly clearer, though the explicit condition also works fine.
forms_pro/utils/test_form_generator.py (1)
226-231: Consider using safer field lookup pattern consistently.The
next()call without a default will raiseStopIterationif the field is missing, which is harder to debug than a clear assertion failure. This pattern is repeated across all four new tests.frontend/src/components/submission/PreviousSubmissionSection.vue (2)
72-76: Consider using named routes for type safety.Direct string interpolation for route construction works but loses type safety and is harder to refactor. Consider using a named route with params.
♻️ Suggested refactor
- @click=" - router.push( - `/p/${submissionFormStore.currentFormRoute}/edit/${submission.name}` - ) - " + @click=" + router.push({ + name: 'PublicEdit', + params: { + route: submissionFormStore.currentFormRoute, + submissionName: submission.name + } + }) + "This requires the route in
router.tsto havename: 'PublicEdit'defined.
32-32: Accordion toggle button lacks accessible label.The chevron button should have an accessible label for screen readers to understand its purpose.
- <Button :icon="isOpen ? 'chevron-up' : 'chevron-down'" variant="outline" /> + <Button + :icon="isOpen ? 'chevron-up' : 'chevron-down'" + variant="outline" + :aria-label="isOpen ? 'Collapse submissions' : 'Expand submissions'" + />frontend/src/pages/submission/PublicEdit.vue (1)
134-154: Simplify redundant disabled logic.The Submit button only renders when
!isDirty(line 135), making the:disabled="isDirty"attribute on line 137 redundant. The button would never be disabled because it only shows when the condition for disabling it is false.♻️ Suggested simplification
<Button v-if="editSubmissionStore.isDraft && !isDirty" label="Submit" - :disabled="isDirty" variant="solid" @click=" () => { submissionFormStore.validateValues(); if (submissionFormStore.errors.length > 0) { return; } editSubmissionStore.submitForm(); } " :loading="editSubmissionStore.submissionResource.loading" - :tooltip=" - isDirty - ? 'You have unsaved changes. Please update the form before submitting.' - : '' - " />forms_pro/api/submission.py (2)
51-83: Remove redundant try/except block.The
try/exceptblock that immediately re-raises the exception provides no value. Per Ruff TRY203/TRY300, the code can be simplified by removing the exception handler entirely.♻️ Proposed refactor
- try: - form: Form = frappe.get_doc("Form", form_id) - linked_doctype = form.linked_doctype - - if not form.is_published: - frappe.throw( - _("This form is un-published, so responses are no longer being collected."), - frappe.PermissionError, - ) - - submission = frappe.new_doc(linked_doctype) - for data in form_data: - submission.set(data["fieldname"], data["value"]) - - submission.fp_linked_form = form_id - submission.fp_submission_status = submission_status.value - submission.insert(ignore_permissions=True) - - # Share the submission with the owner - add_docshare( - doctype=linked_doctype, - name=submission.name, - user=frappe.session.user, - read=1, - write=1, - flags={ - "ignore_share_permission": True, - }, - ) - - return submission.name - except Exception: - raise + form: Form = frappe.get_doc("Form", form_id) + linked_doctype = form.linked_doctype + + if not form.is_published: + frappe.throw( + _("This form is un-published, so responses are no longer being collected."), + frappe.PermissionError, + ) + + submission = frappe.new_doc(linked_doctype) + for data in form_data: + submission.set(data["fieldname"], data["value"]) + + submission.fp_linked_form = form_id + submission.fp_submission_status = submission_status.value + submission.insert(ignore_permissions=True) + + # Share the submission with the owner + add_docshare( + doctype=linked_doctype, + name=submission.name, + user=frappe.session.user, + read=1, + write=1, + flags={ + "ignore_share_permission": True, + }, + ) + + return submission.name
116-124: Docstring is incomplete — missingsubmission_doctypeparameter.The docstring documents
submission_namebut omitssubmission_doctypefrom the Args section.📝 Proposed fix
""" Get a submission by name Args: + submission_doctype: The doctype of the submission submission_name: The name of the submission Returns: The submission """frontend/src/stores/submissionForm.ts (2)
114-117: Validation blocks draft saves — consider allowing incomplete drafts.
saveAsDraftcallsvalidateValues()viasubmitForm(true), which prevents saving if required fields are empty. Typically, drafts should allow incomplete data. Consider skipping validation for drafts.♻️ Proposed approach
async function submitForm(isDraft: boolean = false) { - validateValues(); - if (errors.value.length > 0) { - return; + if (!isDraft) { + validateValues(); + if (errors.value.length > 0) { + return; + } }
150-157: Simplify redundant identity mapping.The
.map((message: string) => message)is an identity operation that can be replaced with a spread or direct assignment.♻️ Proposed fix
onError(error: any) { toast.error("Failed to submit form"); - errors.value = error.messages?.map((message: string) => message) || []; + errors.value = error.messages ? [...error.messages] : []; if (errors.value.length === 0) { errors.value.push( "Error while submitting form. Check the values and try again." ); } },
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
forms_pro/api/form.pyforms_pro/api/submission.pyforms_pro/api/user.pyforms_pro/utils/form_generator.pyforms_pro/utils/test_form_generator.pyfrontend/auto-imports.d.tsfrontend/components.d.tsfrontend/src/components/FormBuilderHeader.vuefrontend/src/components/builder/FieldRenderer.vuefrontend/src/components/submission/FormRenderer.vuefrontend/src/components/submission/FormUnpublishedState.vuefrontend/src/components/submission/PreviousSubmissionSection.vuefrontend/src/index.cssfrontend/src/pages/Dashboard.vuefrontend/src/pages/SubmissionPage.vuefrontend/src/pages/submission/PublicEdit.vuefrontend/src/router.tsfrontend/src/stores/editSubmission.tsfrontend/src/stores/submissionForm.tsfrontend/tailwind.config.ts
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/stores/submissionForm.ts (2)
forms_pro/utils/form_generator.py (1)
SubmissionStatus(62-64)frontend/src/data/session.ts (1)
session(32-59)
🪛 Ruff (0.14.10)
forms_pro/api/submission.py
31-31: Avoid specifying long messages outside the exception class
(TRY003)
81-81: Consider moving this statement to an else block
(TRY300)
82-83: Remove exception handler; error is immediately re-raised
(TRY203)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Server
🔇 Additional comments (30)
frontend/src/components/FormBuilderHeader.vue (1)
3-3: LGTM!The
CloudOffimport is correctly added and used in the template for the unpublished state indicator.frontend/src/index.css (1)
40-42: LGTM!The new
.form-container-simpleclass provides a clean, consistent styling surface for form containers using standard Tailwind utilities.frontend/src/pages/Dashboard.vue (1)
152-152: LGTM!Code style improvement by removing trailing commas. No functional changes.
Also applies to: 162-162
frontend/tailwind.config.ts (1)
13-13: LGTM!The mono font family configuration is correctly defined and extends the Tailwind theme appropriately.
forms_pro/api/form.py (1)
59-59: No changes needed. Thelinked_doctypefield is defined as required (reqd: 1) on the Form doctype and cannot beNone. Direct access without null checks is appropriate and consistent with the rest of the codebase.forms_pro/api/user.py (1)
3-3: LGTM! Type safety improvements.The explicit
Usertype annotation andbool()cast improve type clarity and ensure the response matches theGetUserResponseSchemadefinition.Also applies to: 55-58
frontend/auto-imports.d.ts (1)
8-8: LGTM!Auto-generated file formatting change with no semantic impact.
frontend/src/components/builder/FieldRenderer.vue (1)
15-19: LGTM! Clean disabled state propagation.The
disabledprop is properly defined with a sensible default and consistently propagated to allRenderFieldinstances across the different field type branches.frontend/src/components/submission/PreviousSubmissionSection.vue (1)
1-82: LGTM overall! Well-structured submission history component.Good use of reka-ui accordion components, clean conditional rendering for timestamps, and appropriate status badge styling. The component follows Vue 3 composition API best practices.
forms_pro/utils/form_generator.py (1)
158-175: The suggested fix is incorrect and would break non-custom doctype behavior.The tests confirm that fields must appear in
doctype.fieldsfor both custom and non-custom doctypes. For non-custom doctypes, the code correctly:
- Creates CustomField records (Frappe's mechanism for extending core doctypes)
- Appends to
doctype.fieldsfor the in-memory representation- Saves the doctype
Moving the append/save into an else clause would prevent fields from being added to non-custom doctypes' field lists. The
test_status_field_is_added_core_doctypeandtest_linked_form_field_is_added_core_doctypetests explicitly verify that fields appear indoctype.fieldsfor non-custom doctypes and that CustomField records exist—both conditions that the current implementation satisfies.Likely an incorrect or invalid review comment.
frontend/components.d.ts (1)
6-6: LGTM! Auto-generated component declarations.The additions correctly register the new submission-related components in the global type declarations.
Also applies to: 16-17, 27-27, 29-29
frontend/src/components/submission/FormRenderer.vue (3)
8-15: LGTM! Well-structured prop definition.The disabled prop is properly typed with an appropriate default value.
17-19: LGTM! Clean separation of concerns.Extracting the submit handler improves code organization.
37-52: LGTM! Flexible slot-based architecture.The named slot pattern allows parent components to override the default action buttons while maintaining good defaults.
frontend/src/pages/submission/PublicEdit.vue (5)
16-28: LGTM! Proper async initialization chain.The initialization correctly waits for form resource data before setting up the edit submission store.
45-49: LGTM! Efficient dirty checking.The computed property correctly detects changes by comparing form fields against the original submission.
51-101: LGTM! Clear and informative UI structure.The page header and submission details provide good context for the editing workflow.
102-110: LGTM! Appropriate handling of closed forms.The alert and disabled state clearly communicate that editing is not allowed when the form is closed.
30-43: The watcher is working as designed and doesn't pose a data loss risk. Thesubmissionproperty only updates through explicit user actions (updateForm(),convertToDraft(),submitForm()), not through background polling or auto-refresh. After a user saves changes, syncing the backend's response back to the form fields is the correct behavior, andisDirtycorrectly tracks the state before and after save operations.frontend/src/pages/SubmissionPage.vue (2)
17-28: LGTM! Clear conditional rendering logic.The structure correctly shows:
- Previous submissions (if any exist)
- Either the unpublished state or the active form (mutually exclusive)
The explicit
:disabled="false"binding clarifies intent, though it matches the default value.
15-15: LGTM! Improved layout and positioning.The background styling and fixed branding bar improve the visual hierarchy and user experience.
Also applies to: 30-43
frontend/src/stores/editSubmission.ts (3)
32-46: LGTM! Proper status update with user feedback.The function correctly updates the submission status and provides appropriate toast notifications.
48-57: LGTM! Clean update implementation.The function properly updates submission data with appropriate user feedback.
64-78: LGTM! Proper submission status update.The function correctly updates the submission status to "Submitted" with appropriate user feedback.
forms_pro/api/submission.py (2)
13-31: LGTM on the Pydantic model.The
UserSubmissionResponsemodel is well-structured with proper field definitions, aliasing forfp_submission_status, and a robust datetime validator that handles both string and datetime inputs.One minor note: the static analysis hint about "long messages outside the exception class" (line 31) is a stylistic preference and acceptable here for clarity.
125-131: Permission check implementation looks correct.The function properly validates read permission before returning submission data, which is the right pattern for access control.
frontend/src/stores/submissionForm.ts (4)
50-68: LGTM on user submissions resource setup.The resource configuration with
auto: falseand the computed property handling empty states appropriately.
84-112: Initialization logic is well-structured.The sequential fetch pattern (form data first, then user submissions if logged in) is appropriate. Draft data merging with
useStoragecorrectly preserves user input across sessions.
164-177: Helper functions are correctly implemented.
validateValuesproperly checks required fields, andclearDraftcorrectly clears the localStorage viauseStorage.
179-197: Store API is well-organized.The return object properly exposes all necessary state and methods for the submission workflow.
| submissions = frappe.get_all( | ||
| doctype=linked_doctype, | ||
| filters={"owner": frappe.session.user}, | ||
| fields=["name", "creation", "modified", "fp_submission_status"], | ||
| order_by="creation", | ||
| ) |
There was a problem hiding this comment.
Missing filter for fp_linked_form may return unrelated submissions.
The query retrieves all submissions from linked_doctype owned by the user, but doesn't filter by the specific form. If multiple forms share the same linked_doctype, this will return submissions from all forms rather than just the requested form_id.
🐛 Proposed fix
submissions = frappe.get_all(
doctype=linked_doctype,
- filters={"owner": frappe.session.user},
+ filters={"owner": frappe.session.user, "fp_linked_form": form_id},
fields=["name", "creation", "modified", "fp_submission_status"],
order_by="creation",
)🤖 Prompt for AI Agents
In @forms_pro/api/submission.py around lines 104 - 109, The current
frappe.get_all call for variable submissions queries linked_doctype by owner
only and can return submissions from other forms; update the filters in the
frappe.get_all call (the get_all invocation that uses linked_doctype and fields
["name","creation","modified","fp_submission_status"]) to include fp_linked_form
equal to the requested form_id so only submissions for that specific form are
returned.
| LINKED_FORM_FIELDOPTIONS = { | ||
| "label": "Linked Form", | ||
| "fieldname": "fp_linked_form", | ||
| "fieldtype": "Link", | ||
| "options": "Form", | ||
| "read_only": 1, | ||
| } |
There was a problem hiding this comment.
Missing mandatory key in LINKED_FORM_FIELDOPTIONS.
The test test_linked_form_field_is_added_custom_doctype (line 262) and test_linked_form_field_is_added_core_doctype (line 342) assert linked_form_field.mandatory == LINKED_FORM_FIELDOPTIONS["mandatory"], but this key is not defined in the options dict, which will cause a KeyError.
LINKED_FORM_FIELDOPTIONS = {
"label": "Linked Form",
"fieldname": "fp_linked_form",
"fieldtype": "Link",
"options": "Form",
"read_only": 1,
+ "mandatory": 0,
+ "in_list_view": 0,
}📝 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.
| LINKED_FORM_FIELDOPTIONS = { | |
| "label": "Linked Form", | |
| "fieldname": "fp_linked_form", | |
| "fieldtype": "Link", | |
| "options": "Form", | |
| "read_only": 1, | |
| } | |
| LINKED_FORM_FIELDOPTIONS = { | |
| "label": "Linked Form", | |
| "fieldname": "fp_linked_form", | |
| "fieldtype": "Link", | |
| "options": "Form", | |
| "read_only": 1, | |
| "mandatory": 0, | |
| "in_list_view": 0, | |
| } |
🤖 Prompt for AI Agents
In @forms_pro/utils/form_generator.py around lines 67 - 73, The
LINKED_FORM_FIELDOPTIONS dict is missing the "mandatory" key referenced by
tests; update LINKED_FORM_FIELDOPTIONS (the constant defined in
form_generator.py) to include a "mandatory" entry (e.g., "mandatory": 0) using
the same type convention used elsewhere (int or bool) so
linked_form_field.mandatory matches LINKED_FORM_FIELDOPTIONS["mandatory"] in the
tests.
| linked_form_field = next( | ||
| field | ||
| for field in form_generator.doctype.fields | ||
| if field.fieldname == LINKED_FORM_FIELDOPTIONS["fieldname"] | ||
| ) | ||
| assert linked_form_field is not None, "Linked form field should be added to doctype" | ||
|
|
||
| custom_field_id = frappe.db.exists( | ||
| "Custom Field", | ||
| {"dt": test_doctype.name, "fieldname": LINKED_FORM_FIELDOPTIONS["fieldname"]}, | ||
| ) | ||
| self.assertIsNotNone(custom_field_id, "Custom field should be created for doctype") | ||
|
|
||
| custom_field = frappe.get_doc("Custom Field", custom_field_id) | ||
|
|
||
| self.assertEqual(custom_field.dt, test_doctype.name) | ||
| self.assertEqual(custom_field.fieldname, LINKED_FORM_FIELDOPTIONS["fieldname"]) | ||
| self.assertEqual(custom_field.fieldtype, LINKED_FORM_FIELDOPTIONS["fieldtype"]) | ||
| self.assertEqual(custom_field.options, LINKED_FORM_FIELDOPTIONS["options"]) | ||
| self.assertEqual(custom_field.read_only, LINKED_FORM_FIELDOPTIONS["read_only"]) | ||
| self.assertEqual(custom_field.in_list_view, LINKED_FORM_FIELDOPTIONS["in_list_view"]) | ||
| self.assertEqual(custom_field.mandatory, LINKED_FORM_FIELDOPTIONS["mandatory"]) |
There was a problem hiding this comment.
Same issues in core doctype test.
Lines 341-342 have the same KeyError issue with missing in_list_view and mandatory keys, and lines 321-326 have the same StopIteration risk.
🤖 Prompt for AI Agents
In @forms_pro/utils/test_form_generator.py around lines 321 - 342, Change the
next(...) call that finds linked_form_field to use a safe default (next(...,
None)) and then assert linked_form_field is not None (reference:
linked_form_field and form_generator.doctype.fields) to avoid StopIteration; and
replace direct dict indexing of LINKED_FORM_FIELDOPTIONS for keys like
"in_list_view" and "mandatory" with .get(...) (e.g.,
LINKED_FORM_FIELDOPTIONS.get("in_list_view")) or provide explicit defaults
before asserting equality against custom_field (references:
LINKED_FORM_FIELDOPTIONS and custom_field comparisons) so missing keys no longer
raise KeyError.
| <template> | ||
| <div class="flex flex-col items-center justify-center text-ink-gray-8"> | ||
| <Unplug class="w-10 h-10" /> | ||
| <h1 class="text-2xl font-bold">Form is un-published!</h1> |
There was a problem hiding this comment.
Minor text inconsistency: "un-published" vs "unpublished".
The heading uses "un-published" but standard English uses "unpublished" (no hyphen). Consider updating for consistency.
- <h1 class="text-2xl font-bold">Form is un-published!</h1>
+ <h1 class="text-2xl font-bold">Form is unpublished!</h1>📝 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.
| <h1 class="text-2xl font-bold">Form is un-published!</h1> | |
| <h1 class="text-2xl font-bold">Form is unpublished!</h1> |
🤖 Prompt for AI Agents
In @frontend/src/components/submission/FormUnpublishedState.vue at line 8,
Update the heading text in the FormUnpublishedState.vue component to use the
correct spelling "unpublished" (no hyphen); locate the <h1> element currently
rendering "Form is un-published!" and change it to "Form is unpublished!" to
ensure consistency across the UI.
| const isDraft = computed( | ||
| () => submission.value?.fp_submission_status == "Draft" | ||
| ); | ||
| const isSubmitted = computed( | ||
| () => submission.value?.fp_submission_status == "Submitted" | ||
| ); |
There was a problem hiding this comment.
Use strict equality for status checks.
The status comparisons use loose equality (==) instead of strict equality (===). While this works for string comparisons, strict equality is a best practice to prevent unexpected type coercion.
🔧 Use strict equality
const isDraft = computed(
- () => submission.value?.fp_submission_status == "Draft"
+ () => submission.value?.fp_submission_status === "Draft"
);
const isSubmitted = computed(
- () => submission.value?.fp_submission_status == "Submitted"
+ () => submission.value?.fp_submission_status === "Submitted"
);📝 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.
| const isDraft = computed( | |
| () => submission.value?.fp_submission_status == "Draft" | |
| ); | |
| const isSubmitted = computed( | |
| () => submission.value?.fp_submission_status == "Submitted" | |
| ); | |
| const isDraft = computed( | |
| () => submission.value?.fp_submission_status === "Draft" | |
| ); | |
| const isSubmitted = computed( | |
| () => submission.value?.fp_submission_status === "Submitted" | |
| ); |
🤖 Prompt for AI Agents
In @frontend/src/stores/editSubmission.ts around lines 11 - 16, The computed
status checks use loose equality; update the two computed properties isDraft and
isSubmitted to use strict equality (replace == with ===) when comparing
submission.value?.fp_submission_status to "Draft" and "Submitted" respectively
so the comparisons use === and avoid type coercion.
| async function initialize(doctype: string, name: string) { | ||
| isLoading.value = true; | ||
| submissionDoctype.value = doctype; | ||
| submissionName.value = name; | ||
| submissionResource.value = createDocumentResource({ | ||
| doctype: submissionDoctype.value, | ||
| name: submissionName.value, | ||
| }); | ||
|
|
||
| isLoading.value = false; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find frontend -name "editSubmission.ts" -o -name "editSubmission.js"Repository: BuildWithHussain/forms_pro
Length of output: 107
🏁 Script executed:
cat -n frontend/src/stores/editSubmission.tsRepository: BuildWithHussain/forms_pro
Length of output: 3083
🏁 Script executed:
grep -r "createDocumentResource" frontend/src --include="*.ts" --include="*.js" -B 2 -A 5Repository: BuildWithHussain/forms_pro
Length of output: 2189
🏁 Script executed:
grep -r "isLoading" frontend/src/stores/editSubmission.ts -B 2 -A 2Repository: BuildWithHussain/forms_pro
Length of output: 415
🏁 Script executed:
find frontend -name "PublicEdit.vue" -o -name "PublicEdit.ts"Repository: BuildWithHussain/forms_pro
Length of output: 114
🏁 Script executed:
grep -r "isLoading" frontend/src/components --include="*.vue" -B 2 -A 2Repository: BuildWithHussain/forms_pro
Length of output: 1765
🏁 Script executed:
cat -n frontend/src/stores/editForm.ts | head -80Repository: BuildWithHussain/forms_pro
Length of output: 3170
🏁 Script executed:
grep -r "\.fetch\|\.load" frontend/src/stores --include="*.ts" --include="*.js" -B 1 -A 1Repository: BuildWithHussain/forms_pro
Length of output: 2498
🏁 Script executed:
cat -n frontend/src/pages/submission/PublicEdit.vueRepository: BuildWithHussain/forms_pro
Length of output: 9886
Use the resource's built-in loading state instead of manual flag.
The isLoading flag is set to false immediately after creating the document resource, but createDocumentResource loads data asynchronously. This causes the UI to render (PublicEdit.vue line 52) before submission data is available, creating a race condition where the component accesses submission?.fp_submission_status and other properties before they exist.
Follow the pattern used in editForm.ts and submissionForm.ts: replace the manual isLoading ref with a computed property that tracks the resource's actual loading state:
const isLoading = computed(() => submissionResource.value?.loading || false);Then remove the manual isLoading.value = true/false assignments from the initialize function, or alternatively, await the resource fetch call.
🤖 Prompt for AI Agents
In @frontend/src/stores/editSubmission.ts around lines 20 - 30, Replace the
manual isLoading ref with a computed getter that returns
submissionResource.value?.loading || false, remove the explicit isLoading.value
= true/false assignments inside initialize, and ensure initialize only sets
submissionDoctype, submissionName, and submissionResource via
createDocumentResource; update any references to isLoading to use the new
computed isLoading and rely on submissionResource.loading to reflect async fetch
state (symbols: initialize, isLoading, submissionResource,
createDocumentResource, PublicEdit.vue references).
| export type UserSubmission = { | ||
| name: string; | ||
| creation: string; | ||
| modified: string; | ||
| }; |
There was a problem hiding this comment.
UserSubmission type is missing submission_status field.
The backend UserSubmissionResponse returns submission_status, but the frontend type doesn't include it. This will cause type inconsistencies when consuming the API response.
🐛 Proposed fix
export type UserSubmission = {
name: string;
creation: string;
modified: string;
+ submission_status: string;
};🤖 Prompt for AI Agents
In @frontend/src/stores/submissionForm.ts around lines 9 - 13, The
UserSubmission type is missing the submission_status field; update the exported
type UserSubmission to include submission_status (e.g., submission_status:
string or the appropriate enum/union used by the backend) so the frontend shape
matches the backend UserSubmissionResponse; change any consumers of
UserSubmission to handle the new field where necessary (look for references to
UserSubmission in submissionForm.ts and related selectors/components).
1 participant
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.