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

[3] fix wrong catid for contactcreator #27949

Merged
merged 3 commits into from Feb 20, 2020
Merged

[3] fix wrong catid for contactcreator #27949

merged 3 commits into from Feb 20, 2020

Conversation

alikon
Copy link
Contributor

@alikon alikon commented Feb 16, 2020

Pull Request for Issue #27946 .

Summary of Changes

fixed the wrong category id for Plugin contactcreator

Testing Instructions

Enable the user plugin contactcreator
Enable registering of users in users options
Register as new user.

Expected result

The newly generated contact has categoy "uncategorised" of com_contact.

Actual result

The generated contact has catid = 34

Documentation Changes Required

maybe a better explanation of tefeature

@chmst
Copy link
Contributor

chmst commented Feb 16, 2020

I have tested this item ✅ successfully on c510f9d

Test on mysql.


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

@bembelimen
Copy link
Contributor

I have tested this item 🔴 unsuccessfully on c510f9d

Use case:

  • Delete the category
  • Try again

=> non existing category ID is saved

Potential solution(?): have no value at all and/or check before saving if the category exists.


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

@Bakual
Copy link
Contributor

Bakual commented Feb 16, 2020

@bembelimen This only sets values which are valid for a new installation. As soon as you delete the category or modify stuff, those values aren't valid anymore. That's true for every single value set during installation.
The solution for this "issue" is simple: Check and save the plugin parameters, then it works again.

Potential solution(?): have no value at all and/or check before saving if the category exists.

No value set means the plugin never will work out of the box. With the values set, it works as long as the category isn't deleted, which is probably the case for most sites.
When you edit and save the plugin parameters, you don't have to check if the category exists. It will always only save an existing category.

@richard67
Copy link
Member

@alikon @Bakual Would it make some sense to add some php code to script.php for changing the category value in the params on updates from previous versions where this parameter was wrong? I know we could also do it in update sql with find and replace but that always was a bit dangerous ;-)

@bembelimen Could you change back your test result to not tested with respect to @Bakual 's explanation above?

@Bakual
Copy link
Contributor

Bakual commented Feb 16, 2020

Would it make some sense to add some php code to script.php for changing the category value in the params on updates from previous versions where this parameter was wrong? I know we could also do it in update sql with find and replace but that always was a bit dangerous ;-)

@richard67 I wouldn't do that, and we never did in the past when we had similar issues. People just have to save the plugin parameters to get the correct category, so it's not like it's hard to fix.

@jwaisner
Copy link
Member

I have tested this item ✅ successfully on c510f9d


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

@jwaisner
Copy link
Member

RTC based on 2 successful tests and 1 test that the result was not part of this PR.


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Feb 17, 2020
@Quy Quy added the PR-staging label Feb 17, 2020
@bembelimen
Copy link
Contributor

I still think, that this PR does improve the current status of the code, but does not makes it 100% correct (less wrong is still wrong).

What should happen to make it correct is, that here: https://github.com/joomla/joomla-cms/blob/staging/plugins/user/contactcreator/contactcreator.php#L71-L78 the category should be loaded and checked, if it exists and has the right extension etc. otherwise throw an error.

@Bakual
Copy link
Contributor

Bakual commented Feb 19, 2020

You can of course further improve the code. However that is out of scope for this PR and this PR would still be needed.

@rdeutz rdeutz merged commit 8ea5b9f into joomla:staging Feb 20, 2020
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Feb 20, 2020
@Quy Quy added this to the Joomla! 3.9.16 milestone Feb 20, 2020
@alikon alikon deleted the patch-89 branch February 21, 2020 07:31
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

9 participants