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

Display aliases instead of user logins on annotations #11062 #11653

Closed
wants to merge 10 commits into from

Conversation

quillstuds
Copy link

This pull request replaces the user logins with user aliases in annotations. If a user cannot access the user alias of another user (eg. regular users trying to access a superuser's alias) then annotations fallback to using the user login.

It adds one new private method in the annotations controller to retrieve annotation related user info from the database.

@mattab mattab added this to the 3.0.5 milestone May 16, 2017
@mattab mattab added the Needs Review PRs that need a code review label May 16, 2017
@mattab
Copy link
Member

mattab commented May 16, 2017

Thanks for the pull request @andocobo

Could you please explain a bit why you think this change is useful and makes things better? It seems like a good idea but I can't yet explain why

@quillstuds
Copy link
Author

quillstuds commented May 17, 2017 via email

* Output:
* - List of users with logins and aliases
*/
private function getAnnotationUsers($annotations = '')
Copy link
Member

Choose a reason for hiding this comment

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

You should use an empty array as default for $annotations, otherwise the foreach below might fail

@@ -114,6 +119,8 @@ public function saveAnnotation()
// save the annotation
$view->annotation = Request::processRequest("Annotations.save");

$view->users = $this->getAnnotationUsers();
Copy link
Member

Choose a reason for hiding this comment

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

Any reason for calling this without a parameter? If you want to reset $view->users, simply set it to an empty array.

@sgiehl sgiehl modified the milestones: 3.1.0, 3.0.5 Jul 17, 2017
@mattab
Copy link
Member

mattab commented Sep 21, 2017

Hi @andocobo
Would it be possible that you apply the changes suggested by @sgiehl in the review above? Looking forward to merging this...

@sgiehl
Copy link
Member

sgiehl commented Nov 10, 2018

Closing this one as user aliases are going to be removed

@sgiehl sgiehl closed this Nov 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants