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

User Registration Captcha #38604

Merged
merged 5 commits into from
Oct 23, 2022
Merged

User Registration Captcha #38604

merged 5 commits into from
Oct 23, 2022

Conversation

brianteeman
Copy link
Contributor

Pull Request for Issue #21921 .

Summary of Changes

Moves the captcha to the end of the form. This had already been done for contacts and content

Testing Instructions

  1. Enable recaptcha
  2. set the captcha in global config
  3. enable user registration
  4. create custom user fields for registration
  5. check the registration form on the front end - captcha displayed
  6. disable captcha in global config
  7. check the registration form on the front end - no captcha displayed
  8. enable captcha in the users component
  9. check the registration form on the front end - captcha displayed

Actual result BEFORE applying this Pull Request

the captcha is rendered after the default fields but before any custom fields

Expected result AFTER applying this Pull Request

the captcha is rendered at the end of the form

@crystalenka
Copy link
Member

crystalenka commented Oct 7, 2022

Is this ready for testing?

@brianteeman
Copy link
Contributor Author

Is this ready for testing?

of course. its not a draft - just no one seems interested in testing it and would prefer to complain about the bug

@crystalenka
Copy link
Member

I have tested this item ✅ successfully on 75e0fbe

Works as expected - CAPTCHA shown before custom fields before the PR, and shown after custom fields after the PR was applied. Thanks!


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38604.

@alikon
Copy link
Contributor

alikon commented Oct 8, 2022

I have tested this item ✅ successfully on 75e0fbe


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38604.

@alikon
Copy link
Contributor

alikon commented Oct 8, 2022

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38604.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Oct 8, 2022
@brianteeman
Copy link
Contributor Author

Anything preventing this bug being merged?

@HLeithner HLeithner changed the base branch from 4.2-dev to 4.3-dev October 23, 2022 11:10
@HLeithner HLeithner merged commit 3a86982 into joomla:4.3-dev Oct 23, 2022
@HLeithner
Copy link
Member

thanks

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Oct 23, 2022
@HLeithner HLeithner added this to the Joomla! 4.3.0 milestone Oct 23, 2022
@HLeithner
Copy link
Member

Thanks, I rebased this on 4.3 because it's not a bugfix.

@HLeithner
Copy link
Member

I added the migration documentation at joomla/Manual#52

@brianteeman
Copy link
Contributor Author

of course its a bug fix

@brianteeman brianteeman deleted the captcha branch October 23, 2022 12:05
@crystalenka
Copy link
Member

Would agree this is a bug fix rather than a feature. Captcha should always be rendered right before submit. Even if it wasn't technically "broken" it was a usability bug.

@brianteeman
Copy link
Contributor Author

@crystalenka seems that @HLeithner thinks anything that doesn't spit out an error message is not a bug even if it doesnt do what it says it will do. another example #38953 (comment)

@HLeithner
Copy link
Member

it's not me alone that thinks like this but if you need to blame a person then pick me happy with it.

@brianteeman
Copy link
Contributor Author

Well you are the only one who commented so if anyone else does they are anonymous and it would be impossible for me to know.

The fact still remains that "you" are not being consistent in deciding what is a bug and what is a feature. It just depends on who created the pr.

Looks like we are returning to the bad old days where functionality that doesn't work is never fixed because to fix it is a new feature. And only code from "real" developers is acceptable in this elitist world.

Even when code is contributed specifically at the request of one of these "real" developers it is ignored because the wrong type of person contributed it.

@HLeithner
Copy link
Member

Doesn't matter who provided the PR, maybe you noticed that this PR has been merged already and about 40 other PRs this weekend from various people so please stop attacking me and tell me that I prefer some persons more then other.

thanks

@bembelimen
Copy link
Contributor

Looks like we are returning to the bad old days where functionality that doesn't work is never fixed because to fix it is a new feature. And only code from "real" developers is acceptable in this elitist world.

I just checked the PR and for me it looks like the code is fixed...?

@laoneo
Copy link
Member

laoneo commented Oct 23, 2022

@brianteeman you can also add me to the list of persons who are cautious about merging pr's into a patch branch. Patch releases are really there to stabilize a minor version and should only receive bug fixes (mainly regressions) and not cosmetic stuff. This doesn't mean that they are not relevant, but they should be added to a .0 version where people expect a different behavior, especially in the UI. This is common software development practice and not something Harald and I have invented.

@brianteeman
Copy link
Contributor Author

This was not a cosmetic fix. This was a bug fix. This should have been fixed in 4.2

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.

9 participants