Skip to content

Add password validation checks back to UI#136

Merged
tarekio merged 18 commits intobynt-1387-enforce-password-policiesfrom
BYNT-1386-Add-password-validation-checks-back-to-UI
Jul 22, 2025
Merged

Add password validation checks back to UI#136
tarekio merged 18 commits intobynt-1387-enforce-password-policiesfrom
BYNT-1386-Add-password-validation-checks-back-to-UI

Conversation

@apodacaduron
Copy link
Contributor

@apodacaduron apodacaduron commented Jul 15, 2025

Jira Issue

  1. BYNT-1386

Description

Add password validations checks to change_password.html and users.html

Checklist

  • Tests added/updated
  • Documentation updated (if needed)
  • New strings prepared for translations

API Changes (if applicable)

  • Permissions checked
  • Endpoint tests added

Additional Notes

[Any other relevant information]

@apodacaduron apodacaduron self-assigned this Jul 15, 2025
@apodacaduron apodacaduron changed the title Bynt 1386 add password validation checks back to UI Add password validation checks back to UI Jul 16, 2025
@apodacaduron apodacaduron requested a review from tarekio July 16, 2025 15:37
@apodacaduron apodacaduron marked this pull request as ready for review July 16, 2025 15:37
@tarekio tarekio requested a review from level09 July 17, 2025 11:41
Copy link
Contributor

@tarekio tarekio left a comment

Choose a reason for hiding this comment

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

Good.

Copy link
Collaborator

@level09 level09 left a comment

Choose a reason for hiding this comment

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

This PR is good

</v-container>
</v-main>
<v-card-actions class="pa-4 justify-end">
<v-btn type="submit" variant="elevated" color="primary">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Small suggestion:

I feel we are doing unnecessary loop here, using submit button and submit handler on the form. then blocking submission to validate. instead,

let's remove the submit handler and use a normal button

<v-btn @click="validateAndSave" color="primary" variant="elevated">Save</v-btn>

then

async validateAndSave() {
    const { valid, errors } = await this.$refs.form.validate();
    if (valid) {
        this.save();
    } else {
        this.showSnack("Please review the form for errors.");
        scrollToFirstError(errors);
    }
}

I think this is more vue friendly and simpler.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

const {createApp} = Vue;
const {createVuetify} = Vuetify;
<script>
const { createApp, ref } = Vue;
Copy link
Collaborator

Choose a reason for hiding this comment

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

don't think we need/use ref anywhere

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right it was imported by mistake, fixed

@apodacaduron apodacaduron requested a review from level09 July 21, 2025 14:41
Copy link
Collaborator

@level09 level09 left a comment

Choose a reason for hiding this comment

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

The Button is still using type=submit but we can improve this later

@apodacaduron
Copy link
Contributor Author

@level09 i thought you meant the users form as that actually was using the this.$refs.form.validate(), that's where i changed the type="submit" for @click="validateAndSave", will refactor change_password.html sorry about that

@apodacaduron
Copy link
Contributor Author

@level09 i did a refactor on change_password.html to include the @click and the validateAndSave function

Copy link
Collaborator

@level09 level09 left a comment

Choose a reason for hiding this comment

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

small tweak needed

isStrongPassword: false,
form: {
password: null,
new_password: null,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suggest using '' empty strings rather than null

Null can cause a lot of unexpected scenarios :

// This could throw "Cannot read property 'length' of null"
if (this.form.password.length > 0) { ... }

// Validation rules that expect strings
validationRules.minLength(10)(null) // Could fail unexpectedly


// JSON.stringify behavior differs
JSON.stringify({ password: null })      // {"password":null}
JSON.stringify({ password: "" })        // {"password":""}

also type checking etc etc .. Let's just use empty strings :)

@tarekio tarekio merged commit c67b589 into bynt-1387-enforce-password-policies Jul 22, 2025
2 of 3 checks passed
@tarekio tarekio deleted the BYNT-1386-Add-password-validation-checks-back-to-UI branch July 22, 2025 20:19
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.

3 participants