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

clarification of the return value for the user ID #27374

Merged
merged 6 commits into from
Feb 29, 2020

Conversation

tecpromotion
Copy link
Contributor

@tecpromotion tecpromotion commented Dec 29, 2019

Summary of Changes

To make it clear what the returned number is. It is the user ID. There was some confusion about this in the German language files, and some site operators asked for the unit and thought it was days.

With this change, the username is returned as suggested and the super user is notified.

Testing Instructions

.

Expected result

.

Actual result

.

Documentation Changes Required

@ChristineWk
Copy link

I have tested this item ✅ successfully on 00955a9


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

@brianteeman
Copy link
Contributor

Would it not be better to keep the language string the same and to change the id to username

JText::sprintf('PLG_SYSTEM_PRIVACYCONSENT_NOTIFICATION_USER_PRIVACY_EXPIRED_MESSAGE', $user->user_id)

@tecpromotion tecpromotion changed the title Update en-GB.plg_system_privacyconsent.ini clarification of the return value for the user ID Dec 29, 2019
@tecpromotion
Copy link
Contributor Author

Would it not be better to keep the language string the same and to change the id to username

I've been thinking about that, too. Especially because we (almost) always use the user name in the action logs.

@richard67
Copy link
Member

Would it not be better to keep the language string the same and to change the id to username

I've been thinking about that, too. Especially because we (almost) always use the user name in the action logs.

Agree.

@Quy Quy added the Updates Requested Indicates that this pull request needs an update from the author and should not be tested. label Jan 4, 2020
@zero-24
Copy link
Contributor

zero-24 commented Jan 9, 2020

			$messageModel->notifySuperUsers(
				JText::_('PLG_SYSTEM_PRIVACYCONSENT_NOTIFICATION_USER_PRIVACY_EXPIRED_SUBJECT'),
				JText::sprintf(
					'PLG_SYSTEM_PRIVACYCONSENT_NOTIFICATION_USER_PRIVACY_EXPIRED_MESSAGE',
					JFactory::getUser($user->user_id)->username
				)
			);

https://github.com/joomla/joomla-cms/blob/staging/plugins/system/privacyconsent/privacyconsent.php#L701-L704

This should do the trick. cc @tecpromotion

@joomla-cms-bot joomla-cms-bot removed the Language Change This is for Translators label Jan 11, 2020
@tecpromotion
Copy link
Contributor Author

@brianteeman I have withdrawn the adjustment of the language string and as suggested committed the username as return value. Many thanks to @zero-24 and @richard67

@Quy Quy removed the Updates Requested Indicates that this pull request needs an update from the author and should not be tested. label Jan 13, 2020
@zero-24 zero-24 added this to the Joomla! 3.9.16 milestone Jan 23, 2020
@Quy
Copy link
Contributor

Quy commented Feb 27, 2020

I have tested this item ✅ successfully on 65407b6


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

1 similar comment
@alikon
Copy link
Contributor

alikon commented Feb 29, 2020

I have tested this item ✅ successfully on 65407b6


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

@joomla-cms-bot joomla-cms-bot removed this from the Joomla! 3.9.16 milestone Feb 29, 2020
@alikon
Copy link
Contributor

alikon commented Feb 29, 2020

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Feb 29, 2020
@zero-24 zero-24 added this to the Joomla! 3.9.16 milestone Feb 29, 2020
@zero-24
Copy link
Contributor

zero-24 commented Feb 29, 2020

Merging + cc for @imanickam @infograf768 to check whether this can / should / need to be communicated to the TT's

Thanks @tecpromotion

@zero-24 zero-24 merged commit eb0ce6b into joomla:staging Feb 29, 2020
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Feb 29, 2020
@tecpromotion tecpromotion deleted the patch-3 branch March 2, 2020 17:10
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

8 participants