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 all 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
14 changes: 14 additions & 0 deletions app/adapters/duke-ds-project-permission.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
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.

/*
Project permissions are a nested route under the duke-ds-project route.
There is no top level route to GET an individual permission.
This means querying permissions requires a project id parameter that will be used the build the base url.
*/
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/';
}
});
8 changes: 4 additions & 4 deletions app/components/delivery-breadcrumbs.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ export default Ember.Component.extend({
transfer: null, /* specific transfer to be passed to child breadcrumbs */
selectedRouteName: null, /* route we should select ('deliveries.index', 'deliveries.show', etc) */
isDeliverySelected: Ember.computed('selectedRouteName', function () {
Copy link
Member

Choose a reason for hiding this comment

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

I don't know that a regex is easier to read than indexOf == 0, but FYI there's Ember.computed.match

return this.get('selectedRouteName') !== 'deliveries.index';
// selected route starts with deliveries.show
return this.get('selectedRouteName').indexOf('deliveries.show') == 0;
}),
isResendSelected: Ember.computed('selectedRouteName', function () {
return this.get('selectedRouteName') === 'deliveries.show.resend';
})
isResendSelected: Ember.computed.equal('selectedRouteName', 'deliveries.show.resend'),
isNewDeliverySelected: Ember.computed.equal('selectedRouteName', 'deliveries.new')
});
29 changes: 29 additions & 0 deletions app/components/duke-ds-project-list.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import Ember from 'ember';

const DukeDSProjectList = Ember.Component.extend({
projects: null,
selectionChanged: null, /** action */
columns: [
{
component: "select-row-checkbox",
useFilter: false,
mayBeHidden: false,
className: "select-row-checkbox-column",
},
{
propertyName: "name", title: "Project Name",
className: "duke-ds-project-name"
}
],
actions: {
displayDataChanged: function (e) {
this.get('selectionChanged')(e);
}
}
});

DukeDSProjectList.reopenClass({
positionalParams: ['projects']
});

export default DukeDSProjectList;
35 changes: 35 additions & 0 deletions app/components/duke-ds-user-list.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import Ember from 'ember';

const DukeDSUserList = Ember.Component.extend({
users: null,
multipleSelect: false,
pageSize: 12,
selectionChanged: null, /** action */
columns: [
{
component: "select-row-checkbox",
useFilter: false,
mayBeHidden: false,
className: "select-row-checkbox-column",
},
{ propertyName: "fullName",
title: "Name",
className: "duke-ds-user-fullName",
sortPrecedence: 0
},
{ propertyName: "email",
title: "Email",
className: "duke-ds-user-email"}
],
actions: {
displayDataChanged: function (e) {
this.get('selectionChanged')(e);
}
}
});

DukeDSUserList.reopenClass({
positionalParams: ['users']
});

export default DukeDSUserList;
14 changes: 14 additions & 0 deletions app/components/select-row-checkbox.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// from http://onechiporenko.github.io/ember-models-table/v.2/#/examples/select-rows-with-checkboxes
import Component from '@ember/component';
import {get} from '@ember/object';
import layout from '../templates/components/select-row-checkbox';

export default Component.extend({
layout,
actions: {
clickOnRow(index, record, event) {
get(this, 'clickOnRow')(index, record);
event.stopPropagation();
}
}
});
55 changes: 55 additions & 0 deletions app/controllers/deliveries/new/enter-user-message.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import Ember from 'ember';
import DS from 'ember-data';

export default Ember.Controller.extend({
queryParams: ['projectId', 'toUserId'],
projectId: null,
toUserId: null,
userMessage: null,
application: Ember.inject.controller(),
currentDukeDsUser: Ember.computed.alias('application.currentDukeDsUser'),
project: Ember.computed('projectId', function () {
return DS.PromiseObject.create({
promise: this.get('store').findRecord('duke-ds-project', this.get('projectId'))
});
}),
fromUser: Ember.computed.alias('currentDukeDsUser'),
toUser: Ember.computed('toUserId', function () {
return DS.PromiseObject.create({
promise: this.get('store').findRecord('duke-ds-user', this.get('toUserId'))
});
}),
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() {
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();
},
errorResponse => {
this.setProperties({
errors: errorResponse.errors,
disableNext: false
});
}).then(sentDelivery => {
this.transitionToRoute('deliveries.show', sentDelivery.get('transfer'));
});
}
},
});
56 changes: 56 additions & 0 deletions app/controllers/deliveries/new/select-project.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import Ember from 'ember';
import DS from 'ember-data';

const PROJECT_ADMIN_AUTH_ROLE = 'project_admin';

export default Ember.Controller.extend({
project: null,
application: Ember.inject.controller(),
currentDukeDsUser: Ember.computed.alias('application.currentDukeDsUser'),
currentUserProjectPermission: Ember.computed('project', 'currentDukeDsUser.id', function () {
const projectId = this.get('project.id');
const currentUserId = this.get('currentDukeDsUser.id');
if (!projectId || !currentUserId) {
return null;
}
// Filter permissions for our project and user.
// This should return an array of one item: our user's permissions.
// Return the first item in the array.
return DS.PromiseObject.create({
promise: this.get('store').query('duke-ds-project-permission', {
project: projectId,
user: currentUserId,
}).then(function (permissions) {
return permissions.get('firstObject');
})
});
}),
currentUserCanDeliver: Ember.computed.equal('currentUserProjectPermission.authRole', PROJECT_ADMIN_AUTH_ROLE),
disableNext: Ember.computed.not('currentUserCanDeliver'),
showUserMissingPrivilegesError: Ember.computed('project.id', 'currentUserProjectPermission.authRole', function () {
if (this.get('project.id') == null) {
return false; // do not show error when no project is selected
}
const authRole = this.get('currentUserProjectPermission.authRole');
if (!authRole) {
return false; //do not show error while we are fetching users auth role for this project
}
return authRole != PROJECT_ADMIN_AUTH_ROLE;
}),
actions: {
projectSelectionChanged(actionData) {
this.set('project', actionData.selectedItems.get('firstObject'));
},
back() {
this.transitionToRoute('deliveries');
},
next() {
const projectId = this.get('project.id');
if (projectId) {
if (this.get('currentUserCanDeliver')) {
this.transitionToRoute('deliveries.new.select-recipient', { queryParams: { projectId: projectId }})
}
}
}
},
});
49 changes: 49 additions & 0 deletions app/controllers/deliveries/new/select-recipient.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import Ember from 'ember';
import DS from 'ember-data';

export default Ember.Controller.extend({
queryParams: ['projectId'],
projectId: null,
project: Ember.computed('projectId', function () {
return DS.PromiseObject.create({
promise: this.get('store').findRecord('duke-ds-project', this.get('projectId'))
})
}),
application: Ember.inject.controller(),
currentDukeDsUser: Ember.computed.alias('application.currentDukeDsUser'),
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)')
.rejectBy('email', null);
}),
otherUsersList: Ember.computed('validUsersList.[]', 'currentDukeDsUser', function () {
const currentDukeDSUser = this.get('currentDukeDsUser');
if (currentDukeDSUser) {
return this.get('validUsersList').rejectBy('id', currentDukeDSUser.get('id'));
} else {
return this.get('validUsersList');
}
}),
toUser: null,
disableNext: Ember.computed.not('toUser'),
actions: {
toUserSelectionChanged(actionData) {
this.set('toUser', actionData.selectedItems.get('firstObject'));
},
back() {
this.transitionToRoute('deliveries.new.select-project');
},
next() {
const projectId = this.get('projectId');
const toUserId = this.get('toUser.id');
this.transitionToRoute('deliveries.new.enter-user-message', {
queryParams: {
projectId: projectId,
toUserId: toUserId,
}
});
}
}
});
2 changes: 1 addition & 1 deletion app/models/delivery.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export default DS.Model.extend({
project: DS.belongsTo('DukeDsProject'),
fromUser: DS.belongsTo('DukeDsUser'),
toUser: DS.belongsTo('DukeDsUser'),
state: DS.attr('string'),
state: DS.attr('string', { defaultValue() { return STATE_NEW }}),
transfer: DS.belongsTo('DukeDsProjectTransfer'),
userMessage: DS.attr('string'),
shareUsers: DS.hasMany('DukeDsUser'),
Expand Down
7 changes: 7 additions & 0 deletions app/models/duke-ds-project-permission.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import DS from 'ember-data';

export default DS.Model.extend({
project: DS.belongsTo('DukeDsProject'),
user: DS.belongsTo('DukeDsUser'),
authRole: DS.attr('string')
});
5 changes: 5 additions & 0 deletions app/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ Router.map(function() {
this.route('show', { path: '/:transfer_id'}, function () {
this.route('resend', {});
});
this.route('new', function() {
this.route('select-project');
this.route('select-recipient');
this.route('enter-user-message');
});
});
this.route('get-token');
});
Expand Down
4 changes: 4 additions & 0 deletions app/routes/deliveries/new.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import Ember from 'ember';

export default Ember.Route.extend({
});
9 changes: 9 additions & 0 deletions app/routes/deliveries/new/enter-user-message.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import Ember from 'ember';

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

export default Ember.Route.extend({
beforeModel() {
this.transitionTo('deliveries.new.select-project');
}
});
13 changes: 13 additions & 0 deletions app/routes/deliveries/new/select-project.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import Ember from 'ember';

export default Ember.Route.extend({
model() {
// The duke-ds-project response returns only non-deleted when querying for all records
// however it will return projects that are deleted if queried individually.
// This can cause deleted projects to be included in the project list due to caching in
// ember-data for projects referenced in duke-ds-project-transfers.
return this.get('store').query('duke-ds-project', {
is_deleted: false
});
}
});
7 changes: 7 additions & 0 deletions app/routes/deliveries/new/select-recipient.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import Ember from 'ember';

export default Ember.Route.extend({
model() {
return this.get('store').findAll('duke-ds-user');
}
});
4 changes: 4 additions & 0 deletions app/styles/app.css
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
.small-left-margin {
margin-left: 1em;
}

.select-row-checkbox-column {
width: 2em;
}
4 changes: 4 additions & 0 deletions app/templates/components/delivery-breadcrumbs.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,9 @@
context=transfer
selectedRouteName=selectedRouteName}}
{{/if}}
{{else if isNewDeliverySelected}}
{{breadcrumb-item label="New"
routeName="deliveries.new"
selectedRouteName=selectedRouteName}}
{{/if}}
{{yield}}
4 changes: 3 additions & 1 deletion app/templates/components/delivery-table.hbs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
<h3>Outgoing</h3>
<h3>Outgoing {{#link-to 'deliveries.new' class="btn btn-primary pull-right"}}New Delivery{{/link-to}}</h3>


{{models-table
data=outgoingTransfers
columns=outgoingColumns
Expand Down
13 changes: 13 additions & 0 deletions app/templates/components/duke-ds-project-list.hbs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{{models-table
data=projects
columns=columns
multipleColumnsSorting=false
showColumnsDropdown=false
showGlobalFilter=false
showPageSize=false
pageSize=12
filteringIgnoreCase=true
displayDataChangedAction=(action "displayDataChanged")
}}

{{yield}}
Loading