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

refactor(webapp): captcha to component #754

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Rotzbua
Copy link
Contributor

@Rotzbua Rotzbua commented Jul 8, 2023

Refactor redundant captcha code to one component.

Reference

#752

@Rotzbua

This comment was marked as outdated.

@Rotzbua
Copy link
Contributor Author

Rotzbua commented Jan 11, 2024

@peterthomassen can you test the captcha on components/ActivateAccountActionHandler.vue ?

@peterthomassen
Copy link
Member

Display works fine. Submitting the captcha returns a 400 response with payload

{
    "captcha": {
        "id": [
            "This field is required."
        ],
        "solution": [
            "This field is required."
        ]
    }
}

DOM isn't updated after the 400 response (i.e. spinner moves indefinitely on submission button). Console says:

[Vue warn]: Error in callback for watcher "captcha_error": "TypeError: Cannot read properties of undefined (reading 'getCaptcha')"

found in

---> <ActivateAccountActionHandler> at /usr/src/app/src/components/ActivateAccountActionHandler.vue
       <VForm>
         <VCard>
           <ConfirmationPage> at /usr/src/app/src/views/ConfirmationPage.vue
             <VMain>
               <VApp>
                 <App> at /usr/src/app/src/App.vue
                   <Root>
warn @ chunk-TQVERUCE.js?v=08bf4eec:3674


TypeError: Cannot read properties of undefined (reading 'getCaptcha')
    at VueComponent.captcha_error (ActivateAccountActionHandler.vue:52:1)
    at invokeWithErrorHandling (chunk-TQVERUCE.js?v=08bf4eec:2396:26)
    at Watcher2.run (chunk-TQVERUCE.js?v=08bf4eec:2788:13)
    at flushSchedulerQueue (chunk-TQVERUCE.js?v=08bf4eec:3271:13)
    at Array.<anonymous> (chunk-TQVERUCE.js?v=08bf4eec:2481:12)
    at flushCallbacks (chunk-TQVERUCE.js?v=08bf4eec:2438:14)


[Vue warn]: Error in v-on handler (Promise/async): "TypeError: Spread syntax requires ...iterable[Symbol.iterator] to be a function"

found in

---> <VForm>
       <VCard>
         <ConfirmationPage> at /usr/src/app/src/views/ConfirmationPage.vue
           <VMain>
             <VApp>
               <App> at /usr/src/app/src/App.vue
                 <Root>

@Rotzbua
Copy link
Contributor Author

Rotzbua commented Jan 11, 2024

Thanks for the feedback. Fixed.

The bug "spinner moves indefinitely on submission button" is in the current live version.

Seems the error handling is broken with the error message if the captcha is invalid.

Webapp error:

TypeError: errors[c] is not iterable
    confirm ConfirmationPage.vue:114
    submit ConfirmationPage.vue:88
    VueJS 4
    submit VForm.ts:140
    VueJS 33

Response:

{"captcha":{"non_field_errors":["CAPTCHA could not be validated. Please obtain a new one and try again."]}}

The error is in the live version and not fixed by this PR.

@peterthomassen
Copy link
Member

Would you mind rebasing this? Then I'll take a another look.

@peterthomassen
Copy link
Member

The bug "spinner moves indefinitely on submission button" is in the current live version.

Fair, tracked here: #931

Copy link
Member

@peterthomassen peterthomassen left a comment

Choose a reason for hiding this comment

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

Almost ready! Just two strings have been confused.

Comment on lines +92 to +93
hintAudio: 'Can\'t see? Hear an audio CAPTCHA instead.',
hintImage: 'Trouble hearing? Switch to an image CAPTCHA.',
Copy link
Member

Choose a reason for hiding this comment

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

Those two strings are swapped.

</v-row>
</v-container>
<generic-captcha
@update="(id, solution) => {captchaID=id; captchaSolution=solution}"
Copy link
Member

Choose a reason for hiding this comment

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

nice

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.

None yet

2 participants