feat: hide login button - #33
Conversation
📝 WalkthroughWalkthroughAdds a new whitelisted backend endpoint get_website_settings() that returns a dict with disable_sign_up, and updates the frontend PageHeader to fetch this setting and hide the Login button when disable_sign_up is true. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 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. ✨ Finishing touches
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 |
|
|
||
|
|
||
| @frappe.whitelist(allow_guest=True) | ||
| def get_form_settings() -> dict: |
There was a problem hiding this comment.
get_website_settings
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@frontend/src/components/submission/PageHeader.vue`:
- Around line 44-46: The Login Button is rendered when websiteSettings.data is
still undefined because the current condition
(!websiteSettings.data?.disable_sign_up) treats undefined as allowed; change the
guard to only evaluate after settings are resolved—for example require
websiteSettings.data to be truthy before checking disable_sign_up. Update the
conditional on the Button in PageHeader.vue to ensure websiteSettings.data
exists (e.g., websiteSettings.data && !websiteSettings.data.disable_sign_up or
the component's resolved/loading flag) so the Login button is not shown while
settings are loading; keep the existing redirectToLogin handler unchanged.
| <Button v-else-if="!websiteSettings.data?.disable_sign_up" @click="redirectToLogin"> | ||
| Login | ||
| </Button> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "PageHeader.vue" -type fRepository: BuildWithHussain/forms_pro
Length of output: 121
🌐 Web query:
frappe-ui createResource initial data value behavior undefined null
💡 Result:
Short answer: createResource leaves resource.data undefined until a successful fetch (or until you pass initialData). Set initialData (e.g. initialData: []) or guard for undefined (resource.data || []) and/or check resource.fetched or loading before reading resource.data. This prevents reads of undefined vs null and matches community examples.
References:
- example using initialData with createResource (notifications store). [1]
- forum reports where resource.data remains undefined until fetch completes (discussion and troubleshooting). [2][3]
Sources:
[1] gist: Frappe CRM frontend using createResource with initialData.
[2] Frappe forum: "Getting Undefined In CreateResource from Frappe-UI".
[3] Frappe forum: "Frappe-ui createResource data isn't updating in Vue component".
🏁 Script executed:
cat -n ./frontend/src/components/submission/PageHeader.vue | head -100Repository: BuildWithHussain/forms_pro
Length of output: 2109
Avoid showing Login before settings resolve.
If websiteSettings.data is undefined (before the fetch completes), the condition !websiteSettings.data?.disable_sign_up evaluates to true, causing the Login button to appear even when sign‑up should be disabled. Guard on resolved data to prevent this brief visibility window.
✅ Suggested fix
- <Button v-else-if="!websiteSettings.data?.disable_sign_up" `@click`="redirectToLogin">
+ <Button
+ v-else-if="websiteSettings.data && !websiteSettings.data.disable_sign_up"
+ `@click`="redirectToLogin"
+ >
Login
</Button>📝 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.
| <Button v-else-if="!websiteSettings.data?.disable_sign_up" @click="redirectToLogin"> | |
| Login | |
| </Button> | |
| <Button | |
| v-else-if="websiteSettings.data && !websiteSettings.data.disable_sign_up" | |
| `@click`="redirectToLogin" | |
| > | |
| Login | |
| </Button> |
🤖 Prompt for AI Agents
In `@frontend/src/components/submission/PageHeader.vue` around lines 44 - 46, The
Login Button is rendered when websiteSettings.data is still undefined because
the current condition (!websiteSettings.data?.disable_sign_up) treats undefined
as allowed; change the guard to only evaluate after settings are resolved—for
example require websiteSettings.data to be truthy before checking
disable_sign_up. Update the conditional on the Button in PageHeader.vue to
ensure websiteSettings.data exists (e.g., websiteSettings.data &&
!websiteSettings.data.disable_sign_up or the component's resolved/loading flag)
so the Login button is not shown while settings are loading; keep the existing
redirectToLogin handler unchanged.
Hide Login button if signup is disabled in website settings
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.