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

Adding New User Help Tour and functionality to reset if a user has seen a tour or not #971

Merged
merged 19 commits into from
Jul 12, 2019

Conversation

eiffel777
Copy link
Contributor

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

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project as found in the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@eiffel777 eiffel777 added new feature New functionality Category:User Dashboard Screen shown after user login labels Jun 28, 2019
@eiffel777 eiffel777 added this to the 8.5.0 milestone Jun 28, 2019
@eiffel777 eiffel777 self-assigned this Jun 28, 2019
@@ -0,0 +1,24 @@
<?php
Copy link
Member

@jpwhite4 jpwhite4 Jul 1, 2019

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.

Copy link
Contributor

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);
Copy link
Member

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.

url: XDMoD.REST.url + '/summary/viewedUserTour',
params: {
data: Ext.encode({
uid: CCR.xdmod.ui.mappedPID,
Copy link
Member

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);
Copy link
Member

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']]);
Copy link
Member

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?

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'
Copy link
Member

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));
Copy link
Member

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.

jpwhite4
jpwhite4 previously approved these changes Jul 12, 2019
@eiffel777 eiffel777 merged commit 10fed29 into ubccr:xdmod8.5 Jul 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category:User Dashboard Screen shown after user login new feature New functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants