gocodebox / lifterlms-blocks

WordPress Editor (Gutenberg) blocks for LifterLMS.
4 stars 12 forks source link

Password Field Can be Removed from Password Reusable Block #141

Open nrherron92 opened 3 years ago

nrherron92 commented 3 years ago

Reproduction Steps

image

Expected Behavior

Actual Behavior

Error Messages / Logs


### System and Environment Information

<details>
<summary>System Report</summary>

<!-- Paste your System Report between the three backticks below this line -->

Wordpress

Home Url: https://natalie-test-site.myliftersite.com Site Url: https://natalie-test-site.myliftersite.com Login Url: https://natalie-test-site.myliftersite.com/wp-login.php Version: 5.8.1 Debug Mode: No Debug Log: No Debug Display: Yes Locale: en_US Multisite: No Page For Posts: Not Set Page On Front: Not Set Permalink Structure: /%postname%/ Show On Front: posts Wp Cron: Yes

Settings

Version: 5.4.0 Db Version: 5.4.0 Course Catalog: Course Catalog (#35) [https://natalie-test-site.myliftersite.com/courses/] Membership Catalog: Membership Catalog (#36) [https://natalie-test-site.myliftersite.com/memberships/] Student Dashboard: Dashboard (#38) [https://natalie-test-site.myliftersite.com/dashboard/] Checkout Page: Purchase (#37) [https://natalie-test-site.myliftersite.com/purchase/] Course Catalog Per Page: 9 Course Catalog Sorting: menu_order Membership Catalog Per Page: 9 Membership Catalog Sorting: menu_order Site Membership: Not Set Courses Endpoint: my-courses Edit Endpoint: edit-account Lost Password Endpoint: lost-password Vouchers Endpoint: redeem-voucher Autogenerate Username: no Password Strength Meter: no Minimum Password Strength: Terms Required: no Terms Page: Not Set Checkout Names: Checkout Address: Checkout Phone: Checkout Email Confirmation: no Open Registration: yes Registration Names: Registration Address: Registration Phone: Registration Voucher: Registration Email Confirmation: no Account Names: Account Address: Account Phone: Account Email Confirmation: no Confirmation Endpoint: confirm-payment Force Ssl Checkout: no Country: US Currency: USD Currency Position: left Thousand Separator: , Decimal Separator: . Decimals: 2 Trim Zero Decimals: no Recurring Payments: yes Email From Address: team+sandbox@lifterlms.com Email From Name: A LifterLMS Sandbox Email Footer Text: Email Header Image: Cert Bg Width: 800 Cert Bg Height: 616 Cert Legacy Compat: no

Constants

LLMS_REMOVE_ALL_DATA: undefined LLMS_REST_DISABLE: undefined LLMS_SITE_FEATURE_RECURRING_PAYMENTS: undefined LLMS_SITE_IS_CLONE: undefined

Gateways

Stripe: Enabled Stripe Test Mode: Enabled Stripe Logging: no Stripe Order: 1 Manual: Disabled Manual Logging: Manual Order: 1

Server

Mysql Version: 5.7.35 Php Curl: Yes Php Default Timezone: UTC Php Fsockopen: Yes Php Max Input Vars: 5000 Php Max Upload Size: 512 MB Php Memory Limit: 256M Php Post Max Size: 1024M Php Soap: Yes Php Suhosin: No Php Time Limt: 30 Php Version: 7.3.31 Software: Apache/2.4.51 (Unix) OpenSSL/1.1.1 Wp Memory Limit: 256M

Browser

HTTP USER AGENT: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/94.0.4606.81 Safari/537.36

Theme

Name: Astra Version: 3.7.3 Themeuri: https://wpastra.com/ Authoruri: https://wpastra.com/about/ Template: Child Theme: No Llms Support: Yes

Plugins

LifterLMS: 5.4.0 LifterLMS Stripe Payment Gateway: 5.3.2 Manage User Columns: 1.0.3 User Switching: 1.5.8

Integrations

BbPress: No BuddyPress: No

Template Overrides



</details>

This issue has be recreated:
+ [x] Locally
+ [ ] On a staging site
+ [x] On a production website
+ [ ] With only LifterLMS and a default theme

### Browser, Device, and Operating System Information

+ Browser name and version
+ Operating System name and version
+ Device name and version (if applicable)
thomasplevy commented 3 years ago

@nrherron92

This is expected behavior. We have some amount of validation in place to alert users when required fields are removed but the work stalled after rocco built out a set of functions to ensure that required fields are always displayed (email/password): https://github.com/gocodebox/lifterlms/issues/1589

So if you delete the password block, yes, it will still show on the frontend if it's required to make the form work.

We have plans to build a backend validation API so that if a required field is removed the user is alerted. Currently it only is implemented for the email address field.

I have plans to make it so that passwords are not required (users can create an account without providing a password and they'll get a password reset to confirm their account) but that has not yet been implemented.

eri-trabiccolo commented 3 years ago

@thomasplevy actually the problem is that you can remove a password field from its reusable block, and honestly this makes no sense to me. What's a password reusable block without a password field inside? I think that in the future we should prevent this from happening in the builder.

So if you delete the password block, yes, it will still show on the frontend if it's required to make the form work.

This is not true at the moment because of this bug I'm fixing: https://github.com/gocodebox/lifterlms/issues/1832

:D

thomasplevy commented 3 years ago

Ah yes... I misinterpreted the selected block in the screenshot.

thomasplevy commented 3 years ago

I believe improvements to the required fields API should be the solution to this. Work has been started here but I halted it after the introduction of automatic display of required fields in https://github.com/gocodebox/lifterlms/issues/1589

It seems that this won't be that large an issue when https://github.com/gocodebox/lifterlms/issues/1832 is fixed (@eri-trabiccolo said he's already fixed it)

For the moment I'm going to leave this as low priority.

The reworking of the validation API is stalled out here: https://github.com/gocodebox/lifterlms-blocks/pull/114

A possible solution would be to prevent the removal of the field from a reusable block of the specified type... but... this might be too difficult to actually implement... There's a lot of problems with the usage of reusable blocks... It seemed like the right solution but overcoming some of the shortcomings and potential difficultis is a little tiresome... But in any event, I think with the bugfix @eri-trabiccolo is working on we should be okay.

We can do a LOT to prevent users from breaking their own forms and this is one case where I'm like "Oh... you deleted a field and now the field is missing? That's weird huh..." And yes, I'm saying that a bit sarcastically...