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

[4.0] Add registration menu to login module #26589

Merged
merged 26 commits into from
Oct 25, 2019
Merged

Conversation

Giuse69
Copy link
Contributor

@Giuse69 Giuse69 commented Oct 14, 2019

This PR adds an option to login module to set a menu item for registration page. Useful to override default registration link "index.php?option=com_users&view=registration" to a custom page by its menu item.

Summary of Changes

New option in login module.

Testing Instructions

Apply the PR.

  1. Go to front end: nothing should be changed in registration link in login module.
  2. Edit login module and set a menu item to X: go to front-end and registration link should point to the desired menu item X.

Documentation Changes Required

It's a self-explanatory option, anyway the doc page https://docs.joomla.org/Customising_the_Login_Form_module
may cite the new option.

This PR adds an option to login module to set a menu item for registration page. Useful to override default registration link "index.php?option=com_users&view=registration" to a custom page
@SharkyKZ
Copy link
Contributor

Could you move the code to a helper method so it doesn't clutter the layout? Thanks. See example with return URL

public static function getReturnUrl($params, $type)
.

@joomla-cms-bot joomla-cms-bot changed the title Add registration menu to login module [4.0] Add registration menu to login module Oct 14, 2019
@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-4.0-dev and removed Language Change This is for Translators PR-4.0-dev labels Oct 14, 2019
@infograf768
Copy link
Member

infograf768 commented Oct 14, 2019

Notice: Undefined variable: params in /Applications/MAMP/htdocs/installmulti/joomla40/modules/mod_login/Helper/LoginHelper.php on line 74 Error: Call to a member function get() on null: Call to a member function get() on null

Once $params is added for the method, it also has to be added in login.php

Giuse69 and others added 3 commits October 15, 2019 22:31
Co-Authored-By: SharkyKZ <sharkykz@gmail.com>
Co-Authored-By: SharkyKZ <sharkykz@gmail.com>
Co-Authored-By: SharkyKZ <sharkykz@gmail.com>
@richard67
Copy link
Member

@Giuse69 Could you check and comment @adiheutschi 's negative test result? Did he/she test in a wrong way? Or is it a valid test and you have to correct something?

@MarLinDT
Copy link

I have tested this item ✅ successfully on dd80d52


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

@richard67
Copy link
Member

Guys, 2 successful tests are enough for a Pull Request to get the "Ready to Commit" (RTC) status. As soon as a PR has this, it does not need more tests, and you can focus on other PR's which still need tests.

Here it is more important that we have 1 bad test, and as long as this is not clarified or solved, it can't get RTC.

@adiheutschi
Copy link

Actually, it is not a mistake, the question is whether to link to a menu or Article ... ("Registration Page")???


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

@pascalelocher
Copy link

I have tested this item ✅ successfully on e7a8ef4

PHP Version 7.3.8
Web Server Apache
WebServer to PHP Interface cgi-fcgi
Joomla! Version Joomla! 4.0.0-beta1-dev Development [ Amani ] 17-October-2019 20:21 GMT
User Agent Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:69.0) Gecko/20100101 Firefox/69.0


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

@Giuse69
Copy link
Contributor Author

Giuse69 commented Oct 19, 2019

@adiheutschi The link is to a MENU as for login/logout redirection options that are above and not to an article that less probably can be a registration form (just to explain why a menu item and not an article)

@MarLinDT
Copy link

I have tested this item ✅ successfully on e7a8ef4

Its all fine


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

@richard67
Copy link
Member

The link is to a MENU as for login/logout redirection options that are above and not to an article that less probably can be a registration form (just to explain why a menu item and not an article)

@adiheutschi Is that ok for you? If so, please change your test result in the issue tracker.

@alikon
Copy link
Contributor

alikon commented Oct 19, 2019

RTC #jd19it #pbf19


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Oct 19, 2019
@adiheutschi
Copy link

I have tested this item ✅ successfully on e7a8ef4

Test is successfully

Setting Value
PHP Built On Linux server2.adiheutschi.ch 3.10.0-962.3.2.lve1.5.24.9.el7.x86_64 #1 SMP Wed Feb 13 08:24:50 EST 2019 x86_64
Database Type mysql
Database Version 10.2.27-MariaDB
Database Collation latin1_swedish_ci
Database Connection Collation utf8mb4_general_ci
PHP Version 7.3.10
Web Server LiteSpeed
WebServer to PHP Interface litespeed
Joomla! Version Joomla! 4.0.0-beta1-dev Development [ Amani ] 17-October-2019 20:21 GMT
User Agent Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/77.0.3865.120 Safari/537.36


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

@richard67
Copy link
Member

@adiheutschi No need for more tests, this PR is already RTC.
@Giuse69 No need to update to 4.0-dev as long as no conflicts shown. Just out of date shown is ok.

@richard67
Copy link
Member

richard67 commented Oct 19, 2019

@Giuse69 Don't try to solve PHPCS erros in drone. Currently they fail on all 4.0-dev, it will be fixed soon. Is not related to your PR. But could you explain recent changes? They invalidate previous good tests if these changes are not code style only.

@Giuse69
Copy link
Contributor Author

Giuse69 commented Oct 19, 2019

ok, sorry.... and thanks for information

@richard67
Copy link
Member

To the maintainers: RTC is still ok, recent commits were only merged from 4.0-dev, in one case erroneously reverted but this then fixed later so all is ok now. I've just checked.

@Giuse69
Copy link
Contributor Author

Giuse69 commented Oct 23, 2019

hi, just to understand: after successful tests on PBF 2019 and RTC, it's just a matter of time to have it committed by @wilsonge, correct? thanks

@HLeithner
Copy link
Member

@Giuse69 no not automatically. the PR has to fit in to Joomla! In your case I think it's a good addition and I merge it.

Thanks for your work and thanks to the tester.

@HLeithner HLeithner added this to the Joomla 4.0 milestone Oct 23, 2019
@wilsonge wilsonge merged commit 4ed55bd into joomla:4.0-dev Oct 25, 2019
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Oct 25, 2019
@wilsonge
Copy link
Contributor

I think @HLeithner forgot to hit merge 😆 but what he said - sorry I'm really busy with Brexit related deadlines at work at the moment - so my time for Joomla is low

@HLeithner
Copy link
Member

Lol seams I misses it, thx george

@Giuse69 Giuse69 deleted the Joomla-4.0 branch October 26, 2019 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Language Change This is for Translators
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet