-
Notifications
You must be signed in to change notification settings - Fork 69
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
Adding New User Help Tour and functionality to reset if a user has seen a tour or not #971
Conversation
@@ -0,0 +1,24 @@ | |||
<?php |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought that the controller interface code was deprecated and all new code should use the rest stack instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
^ 100% yes please.
public function setViewedUserTour(Request $request, Application $app) | ||
{ | ||
$user = $this->authorize($request); | ||
$content = json_decode($this->getStringParam($request, 'data', true), true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to properly sanitize use input before storing in the database. This code has a potential code injection vulnerability since the user could put arbitrary content in the 'data' object.
html/gui/js/modules/NoviceUser.js
Outdated
url: XDMoD.REST.url + '/summary/viewedUserTour', | ||
params: { | ||
data: Ext.encode({ | ||
uid: CCR.xdmod.ui.mappedPID, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reason for putting the uid and (transient) token in the data object?
public function setViewedUserTour(Request $request, Application $app) | ||
{ | ||
$user = $this->authorize($request); | ||
$content = json_decode($this->getStringParam($request, 'data', true), true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the complex data encoding? You can simply have a single parameter true / false that would meet the requirements for this end point
} | ||
|
||
$storage = new \UserStorage($user, 'viewed_user_tour'); | ||
$storage->upsert(0, ['uid' => $_POST['uid'], 'viewedTour' => $_POST['viewed_user_tour']]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why store the uid in this data structure?
…ded checks fwhen setting if a user tour is viewed
array( | ||
'success' => true, | ||
'total' => 1, | ||
'message' => 'This user will be now be propmted to view the New User Tour the next time they visit XDMoD' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/propmed/prompted/
{ | ||
$this->authorize($request, array('mgr')); | ||
$viewedTour = $this->getIntParam($request, 'viewedTour', true); | ||
$selected_user = XDUser::getUserByID($this->getIntParam($request, 'uid', true)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should check if $selected_user === null
and throw a badrequest if so.
This PR adds the New User guided tour. When a user logs in a check is made to see if they have viewed the New User tour or not. If they have not, a message box is shown and asks them if they would like to see the New User tour. Along with the option to view or decline the tour a checkbox is also present in the message box asking if they would no longer like to see the 'Do you want to see the New User tour' message box when they log in.
An item is placed in the Help Menu so that anyone can see the help tour at any time no matter if they have seen it before or not.
In the User Management pane in the internal dashboard another option has also been placed in the Actions drop down so that the attribute storing if a person has seen the New User Tour can be reset to false. This value is set to true once a person gets to the last tip in the help tour.
Motivation and Context
Help new users become more familiar with the user interface and functionality of XDMoD
Tests performed
Manually tested in docker
Types of changes
Checklist: