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

Allow users to create deliveries #21

Merged
merged 37 commits into from
Aug 14, 2018
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
7cef82e
creating new deliveries WIP
johnbradley Jul 13, 2018
d52c6c5
Merge branch 'master' of github.com:Duke-GCB/datadelivery-ui into 6-n…
johnbradley Jul 23, 2018
708831c
checkbox for selecting and other cleanup
johnbradley Jul 25, 2018
695fa85
tests for new components
johnbradley Jul 26, 2018
466b672
remove unused model
johnbradley Jul 26, 2018
cb44c00
remove deliveries controller
johnbradley Jul 26, 2018
44781cd
test delivery state field defaults to NEW
johnbradley Jul 26, 2018
50ca344
queries project list with isDeliverable
johnbradley Jul 26, 2018
d5f092f
tests for select-project and select-recipient controllers
johnbradley Jul 26, 2018
dbbe482
Add back in RSVP.all for saveAndSend
johnbradley Jul 26, 2018
d543d4e
duke-ds-project isDeliverable -> is_deliverable
johnbradley Jul 30, 2018
6d77719
Add user instructions to the delivery new subroutes
johnbradley Jul 30, 2018
15da3a8
check perms after user picks a project for delivery
johnbradley Aug 8, 2018
2871f01
force reload of recipient list
johnbradley Aug 8, 2018
068ddab
simplify breadcrumb computed properties
johnbradley Aug 9, 2018
3bf70ad
remove share user selection from select-recipient
johnbradley Aug 9, 2018
2b9d464
fix tests for removed shareUsers
johnbradley Aug 9, 2018
fecb96f
add duke-ds-project model requirement for test
johnbradley Aug 9, 2018
377c191
tweak messaging and layout of screens
johnbradley Aug 9, 2018
193f960
add other model for test
johnbradley Aug 9, 2018
e6ee8e2
filter out bad users and sort by fullName
johnbradley Aug 9, 2018
b493477
test sorting by user Name
johnbradley Aug 9, 2018
ecb4cc2
test changes for filtering out 'null' duke-ds-users
johnbradley Aug 9, 2018
4b821bb
enable case-insensitive projects and users
johnbradley Aug 9, 2018
ff92e30
remove unused queryParam
johnbradley Aug 9, 2018
780c7b9
test for select-project showMissingPrivilegesError
johnbradley Aug 9, 2018
91c45b5
simplify select-project and add more tests
johnbradley Aug 9, 2018
2ddd7d6
add newlines to end of two files
johnbradley Aug 10, 2018
1168694
revert un-necessary pageSize change
johnbradley Aug 10, 2018
ff42d3a
reset errors when existing enter-user-message
johnbradley Aug 10, 2018
cba5f81
minor improvements
johnbradley Aug 10, 2018
189d961
authRole camel case changes
johnbradley Aug 10, 2018
6f229f9
filter out users with empty emails
johnbradley Aug 10, 2018
2b4f7af
simplify controllers with ember shortcuts
johnbradley Aug 10, 2018
310d18d
add comment for duke-ds-project-permissio adapter
johnbradley Aug 13, 2018
34b2205
wrap returned promises in DS.Promise* objects
johnbradley Aug 13, 2018
cb9141d
Update duke-ds-project-permission.js
johnbradley Aug 13, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions app/adapters/duke-ds-project-permission.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import ApplicationAdapter from './application';

export default ApplicationAdapter.extend({
urlForQuery(query) {
Copy link
Member

Choose a reason for hiding this comment

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

I think I follow this, but I wanted to clarify.

In the discussion of Duke-GCB/D4S2#170, I argued that it didn't make sense for the API to have a required query parameter to fetch project permissions. I recognize that, without a proper hasMany relationship to get the related records, Ember Data offers store.query() to do fetch a filtered list for a resource, and this code overrides the default query implementation to work with the project detail route in Duke-GCB/D4S2#170. I think it's probably worth a comment about why this is here.

I'll follow up with an issue, but I wanted to have a discussion on whether or not it makes sense to just make ajax calls for the project permissions instead of shoehorning them into ember data models.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My understanding is the hasMany relationship requires a top level endpoint to get the related keys (ie. /duke-ds-project-permission/<id>). I'm all for just making an ajax call, but not sure how easy it would be to re-use the emberjs auth.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I think we're on the same page - I was mostly trying to think out loud as to why this is done this way. Through the course of that I think I came to the conclusion that this doesn't fit 100% in a vanilla Ember Data store, but there are ways to make an authenticated AJAX call here and customize how the response is handled. We do it in bespin-ui.

Copy link
Member

Choose a reason for hiding this comment

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

See #22

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a comment to the adapter.

const projectId = query.project;
delete query.project; // remove project from query so it will not be appended to the url
return this.buildURL('duke-ds-project') + projectId + '/permissions/';
}
});
10 changes: 8 additions & 2 deletions app/components/duke-ds-user-list.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,14 @@ const DukeDSUserList = Ember.Component.extend({
mayBeHidden: false,
className: "select-row-checkbox-column",
},
{ propertyName: "fullName", title: "Name", className: "duke-ds-user-fullName"},
{ propertyName: "email", title: "Email", className: "duke-ds-user-email"}
{ propertyName: "fullName",
title: "Name",
className: "duke-ds-user-fullName",
sortPrecedence: 0
},
{ propertyName: "email",
title: "Email",
className: "duke-ds-user-email"}
],
actions: {
displayDataChanged: function (e) {
Expand Down
48 changes: 23 additions & 25 deletions app/controllers/deliveries/new/enter-user-message.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import Ember from 'ember';

export default Ember.Controller.extend({
queryParams: ['projectId', 'toUserId', 'shareUserIds'],
queryParams: ['projectId', 'toUserId'],
projectId: null,
toUserId: null,
shareUserIds: null,
userMessage: null,
application: Ember.inject.controller(),
currentDukeDsUser: Ember.computed.alias('application.currentDukeDsUser'),
Expand All @@ -15,36 +14,35 @@ export default Ember.Controller.extend({
toUser: Ember.computed('toUserId', function () {
return this.get('store').findRecord('duke-ds-user', this.get('toUserId'));
}),
shareUsers: Ember.computed('shareUserIds', function () {
const store = this.get('store');
const shareUserIds = this.get('shareUserIds');
if (!shareUserIds) {
return [];
}
return shareUserIds
.split(',')
.map(userId => store.findRecord('duke-ds-user', userId));
}),
errors: null,
errorMessages: Ember.computed.mapBy('errors', 'detail'),
disableNext: false,
actions: {
back() {
const projectId = this.get('projectId');
this.transitionToRoute('deliveries.new.select-recipient', { queryParams: { projectId: projectId }});
},
saveAndSend() {
// shareUsers returns a javascript array of promises, these promises need to be resolved
// into models before we can use them in saving the delivery
return Ember.RSVP.all(this.get('shareUsers')).then((shareUsers) => {
const delivery = this.get('store').createRecord('delivery', {
project: this.get('project'),
fromUser: this.get('fromUser'),
shareUsers: shareUsers,
toUser: this.get('toUser'),
userMessage: this.get('userMessage')
});
return delivery.save();
}).then(savedDelivery => {
this.setProperties({
disableNext: true,
errorMessage: null
});
const delivery = this.get('store').createRecord('delivery', {
project: this.get('project'),
fromUser: this.get('fromUser'),
toUser: this.get('toUser'),
userMessage: this.get('userMessage')
});
return delivery.save().then(
savedDelivery => {
return savedDelivery.send();
}).then(sentDelivery => {
},
errorResponse => {
this.setProperties({
errors: errorResponse.errors,
disableNext: false
});
}).then(sentDelivery => {
this.transitionToRoute('deliveries.show', sentDelivery.get('transfer'));
});
}
Expand Down
9 changes: 4 additions & 5 deletions app/controllers/deliveries/new/select-project.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import Ember from 'ember';

const PROJECT_ADMIN_AUTH_ROLE = 'project_admin';

export default Ember.Controller.extend({
project: null,
application: Ember.inject.controller(),
Expand All @@ -24,7 +26,7 @@ export default Ember.Controller.extend({
}
}),
currentUserCanDeliver: Ember.computed('currentUserProjectAuthRole', function () {
Copy link
Member

Choose a reason for hiding this comment

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

Can't this be Ember.computed.equal()?

return this.get('currentUserProjectAuthRole') == 'project_admin';
return this.get('currentUserProjectAuthRole') == PROJECT_ADMIN_AUTH_ROLE;
}),
disableNext: Ember.computed.not('currentUserCanDeliver'),
showUserMissingPrivilegesError: Ember.computed('project.id', 'currentUserProjectAuthRole', function () {
Expand All @@ -35,7 +37,7 @@ export default Ember.Controller.extend({
if (!authRole) {
return false; //do not show error while we are fetching users auth role for this project
}
return authRole != 'project_admin';
return authRole != PROJECT_ADMIN_AUTH_ROLE;
}),
actions: {
projectSelectionChanged(actionData) {
Expand All @@ -44,7 +46,6 @@ export default Ember.Controller.extend({
selectedItem = actionData.selectedItems[0];
}
this.set('project', selectedItem);
this.set('showMissingPrivilegesError', false);
},
back() {
this.transitionToRoute('deliveries');
Expand All @@ -54,8 +55,6 @@ export default Ember.Controller.extend({
if (projectId) {
if (this.get('currentUserCanDeliver')) {
this.transitionToRoute('deliveries.new.select-recipient', { queryParams: { projectId: projectId }})
} else {
this.set('showMissingPrivilegesError', true);
}
}
}
Expand Down
22 changes: 9 additions & 13 deletions app/controllers/deliveries/new/select-recipient.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,21 @@ export default Ember.Controller.extend({
}),
application: Ember.inject.controller(),
currentDukeDsUser: Ember.computed.alias('application.currentDukeDsUser'),
otherUsersList: Ember.computed('model.[]', 'currentDukeDsUser', function () {
validUsersList: Ember.computed('model.[]', function () {
// remove users with invalid fullName values
return this.get('model')
Copy link
Member

Choose a reason for hiding this comment

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

I see users in my listing with empty email addresses too. Won't this be problematic too?

Copy link
Member

Choose a reason for hiding this comment

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

empty-emails

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added code to filter out users without emails.

.rejectBy('fullName', null)
.rejectBy('fullName', '(null)');
}),
otherUsersList: Ember.computed('validUsersList.[]', 'currentDukeDsUser', function () {
const currentDukeDSUser = this.get('currentDukeDsUser');
if (currentDukeDSUser) {
return this.get('model').rejectBy('id', currentDukeDSUser.get('id'));
return this.get('validUsersList').rejectBy('id', currentDukeDSUser.get('id'));
} else {
return this.get('model');
return this.get('validUsersList');
}
}),
toUser: null,
shareUsers: null,
disableNext: Ember.computed.not('toUser'),
actions: {
toUserSelectionChanged(actionData) {
Expand All @@ -27,25 +32,16 @@ export default Ember.Controller.extend({
}
this.set('toUser', selectedItem);
},
shareUsersSelectionChanged(actionData) {
this.set('shareUsers', actionData.selectedItems);
},
back() {
this.transitionToRoute('deliveries.new.select-project');
},
next() {
const projectId = this.get('projectId');
const toUserId = this.get('toUser.id');
const shareUsers = this.get('shareUsers');
var shareUserIds = null;
if (shareUsers) {
shareUserIds = shareUsers.mapBy('id').join(',');
}
this.transitionToRoute('deliveries.new.enter-user-message', {
queryParams: {
projectId: projectId,
toUserId: toUserId,
shareUserIds: shareUserIds
}
});
}
Expand Down
5 changes: 5 additions & 0 deletions app/routes/deliveries/new/enter-user-message.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
import Ember from 'ember';

export default Ember.Route.extend({
resetController(controller, isExiting) {
if (isExiting) {
controller.set('errors', null);
}
}
});
3 changes: 1 addition & 2 deletions app/routes/deliveries/new/select-recipient.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import Ember from 'ember';

export default Ember.Route.extend({
model() {
// force loading all list of users so they don't start filling in after a few seconds
return this.get('store').findAll('duke-ds-user', {reload: true});
return this.get('store').findAll('duke-ds-user');
}
});
2 changes: 1 addition & 1 deletion app/templates/components/delivery-table.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,5 @@
showColumnsDropdown=false
showGlobalFilter=false
showPageSize=false
pageSize=12
pageSize=20
}}
1 change: 1 addition & 0 deletions app/templates/components/duke-ds-project-list.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
showGlobalFilter=false
showPageSize=false
pageSize=12
filteringIgnoreCase=true
Copy link
Member

Choose a reason for hiding this comment

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

Wish I had known about this before!

displayDataChangedAction=(action "displayDataChanged")
}}

Expand Down
1 change: 1 addition & 0 deletions app/templates/components/duke-ds-user-list.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
showGlobalFilter=false
showPageSize=false
pageSize=pageSize
filteringIgnoreCase=true
displayDataChangedAction=(action "displayDataChanged")
multipleSelect=multipleSelect
}}
Expand Down
2 changes: 1 addition & 1 deletion app/templates/components/select-row-checkbox.hbs
Original file line number Diff line number Diff line change
@@ -1 +1 @@
<span class="{{if isSelected themeInstance.select-row themeInstance.deselect-row}}" onclick={{action "clickOnRow" index record}}></span>
<span class="{{if isSelected themeInstance.select-row themeInstance.deselect-row}}" onclick={{action "clickOnRow" index record}}></span>
2 changes: 1 addition & 1 deletion app/templates/deliveries/index.hbs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{{delivery-breadcrumbs selectedRouteName="deliveries.index"}}
<h2>All Deliveries</h2>

{{delivery-table model currentDukeDsUser}}
{{delivery-table model currentDukeDsUser}}
13 changes: 5 additions & 8 deletions app/templates/deliveries/new/enter-user-message.hbs
Original file line number Diff line number Diff line change
@@ -1,23 +1,21 @@
{{delivery-breadcrumbs selectedRouteName="deliveries.new"}}
<h3>New Delivery</h3>
<p class="body">
Click the <strong>Send Delivery</strong> button to begin delivery of
<b>{{project.name}}</b> to <b>{{toUser.fullName}}</b>.
</p>
<h4>
Click <strong>Send Delivery</strong> to begin transferring ownership of your project.
</h4>
<p class="body">
This will send an email to the recipient that includes a link for the recipient to accept or delcine the delivery.
If the user accepts the delivery they will take ownership of the project.
Once the delivery is accepted you will retain read-only permissions.
If you wish to add or edit a custom message (e.g. details about this specific delivery), please enter those notes in the
<strong>User Message</strong> area.
<strong>User Message</strong> area below.
</p>
<div class="panel panel-default">
<div class="panel-heading">
<h3 class="panel-title">Project: {{project.name}} </h3>
</div>
<div class="panel-body">
<dl class="dl-horizontal">

{{#detail-label-value label="From"}}{{fromUser.fullName}}{{/detail-label-value}}
{{#detail-label-value label="To"}}{{toUser.fullName}}{{/detail-label-value}}
{{#each shareUsers as |shareUser index|}}
Expand All @@ -28,16 +26,15 @@
{{/if}}
{{/each}}
{{#detail-label-value label="User Message"}}{{user-message-textarea value=userMessage readonly=false}}{{/detail-label-value}}

</dl>
</div>
</div>

{{error-message-alert errorMessages}}

{{#bs-button type="default" onClick=(action 'back') class="back-button"}}Back{{/bs-button}}

{{#bs-button type="primary" disabled=disableNext onClick=(action 'saveAndSend') class="pull-right next-button"}}
Send Delivery
{{/bs-button}}

{{outlet}}
5 changes: 1 addition & 4 deletions app/templates/deliveries/new/select-project.hbs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
{{delivery-breadcrumbs selectedRouteName="deliveries.new"}}
<h1>New Delivery</h1>
<p class="body">
Select a project to deliver to another user.
</p>
<h4>Select a project to deliver to another user.</h4>
<p class="body">
This will transfer ownership of the project to another user.
Once the recipient accepts the delivery you will retain read-only permissions.
Expand All @@ -17,7 +15,6 @@
{{duke-ds-project-list model selectionChanged=(action 'projectSelectionChanged')}}

{{#bs-button type="default" onClick=(action 'back') class="back-button"}}Back{{/bs-button}}

{{#bs-button type="primary" disabled=disableNext onClick=(action 'next') class="pull-right next-button"}}Next{{/bs-button}}

{{outlet}}
16 changes: 5 additions & 11 deletions app/templates/deliveries/new/select-recipient.hbs
Original file line number Diff line number Diff line change
@@ -1,19 +1,13 @@
{{delivery-breadcrumbs selectedRouteName="deliveries.new"}}
<h1>New Delivery</h1>

<p class="body">
Select the recipient you wish to transfer ownership of <b>{{project.name}}</b> to.
</p>
<h4>
Select the user you wish to transfer ownership of <b>{{project.name}}</b> to.
</h4>
<p class="body">
This selected user will take ownership of the project once the delivery is accepted.
The selected user will take ownership of the project once the delivery is accepted.
Once the delivery is accepted you will retain read-only permissions.
</p>
{{duke-ds-user-list otherUsersList pageSize=6 selectionChanged=(action 'toUserSelectionChanged')}}

<p class="body">
Select additional users to share project.
</p>
{{duke-ds-user-list otherUsersList pageSize=6 multipleSelect=true selectionChanged=(action 'shareUsersSelectionChanged')}}
{{duke-ds-user-list otherUsersList pageSize=12 selectionChanged=(action 'toUserSelectionChanged')}}

{{#bs-button type="default" onClick=(action 'back') class="back-button"}}Back{{/bs-button}}
{{#bs-button type="primary" disabled=disableNext onClick=(action 'next') class="pull-right next-button"}}Next{{/bs-button}}
Expand Down
25 changes: 15 additions & 10 deletions tests/integration/components/duke-ds-user-list-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,22 @@ moduleForComponent('duke-ds-user-list', 'Integration | Component | duke ds user
integration: true
});

test('it renders a list of users', function(assert) {
test('it renders a list of users sorted by fullName', function(assert) {
this.set('users', [
{fullName: 'Joe', email: 'joe@joe.org'},
{fullName: 'Jim', email: 'jim@jim.org'},
{id:'1', fullName: 'Joe', email: 'joe@joe.org'},
{id:'2', fullName: 'Jim', email: 'jim@jim.org'},
]);
this.set('externalAction', function () {});
this.render(hbs`{{duke-ds-user-list users selectionChanged=(action externalAction)}}`);

this.render(hbs`{{duke-ds-user-list users}}`);
//assert.equal(this.$().html().trim(), 'Project Name');
assert.equal(this.$('.duke-ds-user-fullName').eq(0).text().trim(), 'Name'); //header
assert.equal(this.$('.duke-ds-user-email').eq(0).text().trim(), 'Email'); //header
// index 1 is the search box
assert.equal(this.$('.duke-ds-user-fullName').eq(2).text().trim(), 'Joe');
assert.equal(this.$('.duke-ds-user-email').eq(2).text().trim(), 'joe@joe.org');
assert.equal(this.$('.duke-ds-user-fullName').eq(3).text().trim(), 'Jim');
assert.equal(this.$('.duke-ds-user-email').eq(3).text().trim(), 'jim@jim.org');
assert.equal(this.$('.duke-ds-user-fullName').eq(2).text().trim(), 'Jim');
assert.equal(this.$('.duke-ds-user-email').eq(2).text().trim(), 'jim@jim.org');
assert.equal(this.$('.duke-ds-user-fullName').eq(3).text().trim(), 'Joe');
assert.equal(this.$('.duke-ds-user-email').eq(3).text().trim(), 'joe@joe.org');
});

test('it sends selected project to selectionChanged action', function(assert) {
Expand All @@ -28,7 +29,11 @@ test('it sends selected project to selectionChanged action', function(assert) {
{fullName: 'Joe', email: 'joe@joe.org'},
{fullName: 'Jim', email: 'jim@jim.org'},
]);
this.set('externalAction', (actionData) => assert.equal(actionData.selectedItems[0].fullName, 'Jim'));
this.set('externalAction', function (actionData) {
if (actionData.selectedItems[0]) {
assert.equal(actionData.selectedItems[0].fullName, 'Jim');
}
});
this.render(hbs`{{duke-ds-user-list users selectionChanged=(action externalAction)}}`);
this.$('.duke-ds-user-fullName').eq(3).click(); //click Jim row
this.$('.duke-ds-user-fullName').eq(2).click(); //click Jim row
});
12 changes: 12 additions & 0 deletions tests/unit/adapters/duke-ds-project-permission-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { moduleFor, test } from 'ember-qunit';

moduleFor('adapter:duke-ds-project-permission', 'Unit | Adapter | duke ds project permission', {
// Specify the other units that are required for this test.
needs: ['service:session']
});

test('it returns duke-ds-project permisions sub route for urlForQuery', function(assert) {
let adapter = this.subject();
let url = adapter.urlForQuery({project: 'project1'}, 'duke-ds-project-permission');
assert.equal(url, 'http://testhost/duke-ds-projects/project1/permissions/')
});
Loading