Skip to content
This repository was archived by the owner on Sep 8, 2020. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 2 additions & 4 deletions modules/App.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
var module = angular.module('App', ['ui.router', 'App.Authentication', 'App.Guest']);


module.config( ($urlRouterProvider, $rootScope) => {

module.config(function($urlRouterProvider, $rootScope) {
// Default URL if no matches
$urlRouterProvider.otherwise("/projects");
$urlRouterProvider.otherwise('/projects');

// Global catching of uiRouter errors (for development)
$rootScope.$on('stateChangeError', (event, toState, toParams, fromState, fromParams, error) => {
console.log( event, toState, toParams, fromState, fromParams, error );
});

});
34 changes: 16 additions & 18 deletions modules/Authentication/Authenticated.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,31 +5,29 @@ Authentication Module
Most of the actual app is located under here. This abstract module primarily tackles authenticating the user
*/

var module = angular.module('App.Authenticated', ['ui.router'])
var module = angular.module('App.Authenticated', ['ui.router']);

module.config(function($stateProvider)
module.config(function($stateProvider) {
$stateProvider.state('authenticated', {
templateUrl: 'modules/Authenticated/Authenticated.html',
abstract: true,
resolve: {
user: (User, Authentication, $state, $q) => {
return Authentication.checkCredentials().then((response) => {
return new User(response.data);
}, (error) => {
// must return a rejected promise in order to stay in rejected-mode
return $q.reject( $state.go('login') );
});
},
// layout variable for breadcrumb nav (populated by children)
breadcrumbs: () => {
return [];
}
user: (User, Authentication, $state, $q) => {
return Authentication.checkCredentials()
.then(
(response) => new User(response.data),
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure this will get parsed properly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

quite sure :)

// must return a rejected promise in order to stay in rejected-mode
(error) => $q.reject($state.go('login'))
);
},
// layout variable for breadcrumb nav (populated by children)
breadcrumbs: () => []
},
onEnter: (user) => {
user.open();
onEnter(user) {
Copy link
Member

Choose a reason for hiding this comment

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

This isn't a class, this syntax won't work I don't think in the middle of a simple hash object.

Copy link
Member

Choose a reason for hiding this comment

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

NVM just learned this works and is kickass

user.open();
},
onExit: (user) => {
user.close();
onExit(user) {
user.close();
}
});
});
31 changes: 12 additions & 19 deletions modules/Authentication/Login.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,51 +9,44 @@ $modal is part of ui.bootstrap
// Tracks the previous location and allows you to redirect back to that location
var previousLocation;

var module = angular.module('App.Login', ['ui.router', 'ui.bootstrap'])
var module = angular.module('App.Login', ['ui.router', 'ui.bootstrap']);

module.config( ($stateProvider) => {
module.config(function($stateProvider) {
$stateProvider.state('login', {
parent: 'guest',
url: '/login',
resolve: {
user: (User) => {
return new User();
},
user: (User) => new User(),
Copy link
Member

Choose a reason for hiding this comment

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

Lets keep syntaxes consistent, although we're just making it harder and harder for peopel to read this lol.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should revert the above version and just do it all at once? Geez so much syntax sugar I don't even know what to choose anymore lol.

redirect: ($location) => {
return () => {
// Note we're returning a function to be called later, that has the redirect info pre-filled
// TODO: Change to state code
$location.path( previousLocation );
};
},
modal: ($modal) => {
return $modal.open({
templateUrl: "modules/Authentication/Login.html",
controller: 'Login',
});
}
modal: ($modal) => $modal.open({
templateUrl: 'modules/Authentication/Login.html',
controller: 'Login',
})
},
onExit: (modal) => {
onExit(modal) {
modal.close();
}
});
});

module.run( ($rootScope, $location) => {
module.run(function($rootScope, $location) {
$rootScope.$on('$stateChangeStart', (event, toState, toParams, fromState, fromParams) => {
// TODO: Change to state code
previousLocation = $location.path()
previousLocation = $location.path();
});
});

module.controller('Login', ($scope, UserObject, Authentication, user, redirect) => {
$scope.user = user;

$scope.login = () => {
Authentication.login($scope.user, $scope.rememberMe).then( () => {
redirect();
}, (response) => {
$scope.error = response;
});
Authentication.login($scope.user, $scope.rememberMe)
.then( redirect, (response) => $scope.error = response);
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 if this level of compactness is a good thing.

};
});
2 changes: 1 addition & 1 deletion modules/Guest/Guest.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
var module = angular.module('App.Guest', ['ui.router']);

module.config( ($stateProvider) => {
module.config(function($stateProvider) {
$stateProvider.state('guest', {
templateUrl: 'modules/Guest/Guest.html',
abstract: true
Expand Down
15 changes: 10 additions & 5 deletions modules/Object.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
var module = angular.module('App');

module.factory('BaseObject', () => {

class BaseObject {
constructor(data = {}) {
for (property in data) {
this[property] = data[property];
}
/*
Creates a new instance from JSON.
*/
static new(data) {
Copy link
Member

Choose a reason for hiding this comment

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

Yay. But docblock should just be changed to alternative tonew Object()` and an example of how you would use it like you showed me.

return new this(data);
}
Copy link
Member

Choose a reason for hiding this comment

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

What's the purpose of this? I don't know if I agree with this degree of obfuscation (I started to do this stuff in earlier versions in methods called newProject() etc and took it out because it just ended up making it harder to explain what's going on to people. I want people to use the new Project() in their code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

readability

turns things like

return $http.get('/api/projects', { params: { user_id: userId } }).then( (response) => {
  return response.data.map( (project) => {
    return new Project(project);
  });
});

into

return $http.get('/api/projects', { params: { user_id: userId } })
  .then( (response) => response.data.map(Project.fromJSON));

to make it stand out even more:

.then( (response) => {
  return response.data.map( (project) => {
    return new Project(project);
  });
});
.then( (response) => response.data.map(Project.fromJSON));

Copy link
Member

Choose a reason for hiding this comment

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

Maybe lets call it Project.new() instead? I'd still prefer to use the original method, even if it is more verbose (at least for now):

response.data.map( data => new Project(data) )

Copy link
Member

Choose a reason for hiding this comment

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

This is an interesting point you make, I just like Project.new() better because it's more self-evident that this is just an alternative syntax to new Project()



constructor(data) {
Object.assign(this, data);
}

/*
Expand Down
32 changes: 16 additions & 16 deletions modules/Paginator.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
/**
* Paginator - Simple paginator utility example that abstracts logic in a controllable pattern
*
*
* @param paginate {function} Query function that takes paginationOptions
*
*
* @example
* resolve: {
* // Prepares the paginator
Expand All @@ -15,7 +15,7 @@
* return paginator.next();
* }
* }
*
*
* @example
* resolve: {
* taskPaginator: function(Paginator, Task, $stateParams) {
Expand All @@ -29,9 +29,7 @@
* }
*/
angular.module('App').factory('Paginator', function($q){

class Paginator {

constructor(paginate, options) {
this.paginate = paginate;
this.items = [];
Expand All @@ -42,24 +40,26 @@ angular.module('App').factory('Paginator', function($q){
offset: 0
}, options);
}

next() {
if (this.hasMore) {
this.loading = true;

return this.paginate(this.options).then((response) => {
//If the results are less than the required limit then the results are finished
this.hasMore = response.data.results.length >= this.options.limit;

this.items = this.items.concat(response.data.results);
this.options.offset = this.items.length;
return this.items;
}).finally(() => this.loading = false);

return this.paginate(this.options)
.then((response) => {
//If the results are less than the required limit then the results are finished
this.hasMore = response.data.results.length >= this.options.limit;

this.items = this.items.concat(response.data.results);
this.options.offset = this.items.length;

return this.items;
})
.finally(() => this.loading = false);
} else {
return $q.when(this.items);
}
}

}

return Paginator;
Expand Down
60 changes: 23 additions & 37 deletions modules/Project/Project.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,39 +4,35 @@ Project Module
*/
var module = angular.module('App.Project', ['ui.router', 'ui.bootstrap'])

module.config( ($stateProvider) => {
module.config(function($stateProvider) {
$stateProvider.state( 'projects', {
parent: 'authenticated',
url: '/projects',
templateUrl: 'modules/Project/Projects.html',
controller: 'Projects',
resolve: {
projects: (authenticatedUser, Project) => {
return Project.list(authenticatedUser.id);
}
projects: (authenticatedUser, Project) => Project.list(authenticatedUser.id)
},
// `breadcrumbs` resolved in `authenticated` state
onEnter: function(breadcrumbs) {
onEnter(breadcrumbs) {
breadcrumbs.push({ label: 'Projects', sref: 'projects' });
},
onExit: function(breadcrumbs) {
onExit(breadcrumbs) {
breadcrumbs.pop();
},
});
$stateProvider.state( 'projects.new', {
url: '/new', // /projects/new (state must be defined BEFORE /:projectId)
resolve: {
project: (authenticatedUser, Project) => {
return new Project({ user_id: authenticatedUser.id });
}
project: (authenticatedUser, Project) => new Project({ user_id: authenticatedUser.id })
},
templateUrl: 'modules/Project/Form.html',
controller: 'ProjectForm',
// `breadcrumbs` resolved in `authenticated` state
onEnter: function(breadcrumbs) {
onEnter(breadcrumbs) {
breadcrumbs.push({ label: 'New', sref: 'projects.new' });
},
onExit: function(breadcrumbs) {
onExit(breadcrumbs) {
breadcrumbs.pop();
}
});
Expand All @@ -54,15 +50,13 @@ module.config( ($stateProvider) => {
}
},
resolve: {
project: ($stateParams, Project) => {
return Project.get($stateParams.projectId);
}
project: ($stateParams, Project) => Project.get($stateParams.projectId)
},
onEnter: (project, breadcrumbs) => {
onEnter(project, breadcrumbs) {
project.open();
breadcrumbs.push({ label: project.name, sref: 'project' }); // Params inferred when going up
},
onExit: (project, breadcrumbs) => {
onExit(project, breadcrumbs) {
project.close();
breadcrumbs.pop();
}
Expand Down Expand Up @@ -92,37 +86,29 @@ module.controller( 'ProjectForm', ($scope, project) => {

module.factory('ProjectObject', (BaseObject, $http) => {
class Project extends BaseObject {

static list(userId) {
return $http.get('/api/projects', { params: { user_id: userId } }).then( (response) => {
return response.data.map( (project) => {
return new Project(project);
});
});
return $http.get('/api/projects', { params: { user_id: userId } })
.then( (response) => response.data.map(Project.new));
}

static get(id) {
return $http.get('/api/projects/' + id).then( (response) => {
return new Project(response.data);
});
return $http.get(`/api/projects/${id}`)
.then( (response) => new Project(response.data));
}


create() {
return $http.post('/api/projects', this ).then( (response) => {
this.id = response.data.id;
return response.data;
});
return $http.post('/api/projects', this )
.then( (response) => {
this.id = response.data.id;
return response.data;
Copy link
Member

Choose a reason for hiding this comment

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

I agree using Object.assign() is probably better here. Go ahead, I just primarily wanted to bring attention to the important property we ACTUALLY need. I would at least point out in a 1 line comment that we want at least the id propagated to the original instance.

});
}

update() {
return $http.put('/api/projects/' + this.id, this );
}

close() {
super.close();
return $http.put(`/api/projects/${this.id}`, this);
}
}

return ProjectObject;
return Project;
});
Loading