-
Notifications
You must be signed in to change notification settings - Fork 2k
Article Refactor #874
Article Refactor #874
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,7 @@ | ||
'use strict'; | ||
(function (app) { | ||
'use strict'; | ||
|
||
// Use Applicaion configuration module to register a new module | ||
ApplicationConfiguration.registerModule('articles'); | ||
app.registerModule('articles'); | ||
app.registerModule('articles.models'); | ||
app.registerModule('articles.routes', ['ui.router', 'articles.models']); | ||
})(ApplicationConfiguration); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,13 @@ | ||
'use strict'; | ||
(function () { | ||
'use strict'; | ||
|
||
// Setting up route | ||
angular.module('articles').config(['$stateProvider', | ||
function ($stateProvider) { | ||
// Articles state routing | ||
angular | ||
.module('articles.routes') | ||
.config(routeConfig); | ||
|
||
routeConfig.$inject = ['$stateProvider']; | ||
|
||
function routeConfig($stateProvider) { | ||
$stateProvider | ||
.state('articles', { | ||
abstract: true, | ||
|
@@ -12,25 +16,59 @@ angular.module('articles').config(['$stateProvider', | |
}) | ||
.state('articles.list', { | ||
url: '', | ||
templateUrl: 'modules/articles/client/views/list-articles.client.view.html' | ||
templateUrl: 'modules/articles/client/views/list-articles.client.view.html', | ||
controller: 'ArticlesListController', | ||
controllerAs: 'vm' | ||
}) | ||
.state('articles.create', { | ||
url: '/create', | ||
templateUrl: 'modules/articles/client/views/create-article.client.view.html', | ||
templateUrl: 'modules/articles/client/views/form-article.client.view.html', | ||
controller: 'ArticlesController', | ||
controllerAs: 'vm', | ||
resolve: { | ||
articleResolve: newArticle | ||
}, | ||
data: { | ||
roles: ['user', 'admin'] | ||
}, | ||
resolve: { | ||
articleResolve: newArticle | ||
} | ||
}) | ||
.state('articles.view', { | ||
url: '/:articleId', | ||
templateUrl: 'modules/articles/client/views/view-article.client.view.html' | ||
}) | ||
.state('articles.edit', { | ||
url: '/:articleId/edit', | ||
templateUrl: 'modules/articles/client/views/edit-article.client.view.html', | ||
templateUrl: 'modules/articles/client/views/form-article.client.view.html', | ||
controller: 'ArticlesController', | ||
controllerAs: 'vm', | ||
resolve: { | ||
articleResolve: getArticle | ||
}, | ||
data: { | ||
roles: ['user', 'admin'] | ||
} | ||
}) | ||
.state('articles.view', { | ||
url: '/:articleId', | ||
templateUrl: 'modules/articles/client/views/view-article.client.view.html', | ||
controller: 'ArticlesController', | ||
controllerAs: 'vm', | ||
resolve: { | ||
articleResolve: getArticle | ||
} | ||
}); | ||
} | ||
]); | ||
|
||
getArticle.$inject = ['$stateParams', 'Article']; | ||
|
||
function getArticle($stateParams, Article) { | ||
return Article.get({ | ||
articleId: $stateParams.articleId | ||
}).$promise; | ||
} | ||
|
||
newArticle.$inject = ['Article']; | ||
|
||
function newArticle(Article) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do this? Why not just do it in the controller There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ilanbiala It makes the controller smaller and allows the use of UI-Router nested views to share the resolve between the views and other controllers. |
||
return new Article(); | ||
} | ||
})(); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,84 +1,52 @@ | ||
'use strict'; | ||
(function () { | ||
'use strict'; | ||
|
||
// Articles controller | ||
angular.module('articles').controller('ArticlesController', ['$scope', '$stateParams', '$location', 'Authentication', 'Articles', | ||
function ($scope, $stateParams, $location, Authentication, Articles) { | ||
$scope.authentication = Authentication; | ||
angular | ||
.module('articles') | ||
.controller('ArticlesController', ArticlesController); | ||
|
||
// Create new Article | ||
$scope.create = function (isValid) { | ||
$scope.error = null; | ||
ArticlesController.$inject = ['$scope', '$state', 'articleResolve', 'Authentication']; | ||
|
||
if (!isValid) { | ||
$scope.$broadcast('show-errors-check-validity', 'articleForm'); | ||
|
||
return false; | ||
} | ||
|
||
// Create new Article object | ||
var article = new Articles({ | ||
title: this.title, | ||
content: this.content | ||
}); | ||
function ArticlesController($scope, $state, article, Authentication) { | ||
var vm = this; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is this necessary? |
||
|
||
// Redirect after save | ||
article.$save(function (response) { | ||
$location.path('articles/' + response._id); | ||
|
||
// Clear form fields | ||
$scope.title = ''; | ||
$scope.content = ''; | ||
}, function (errorResponse) { | ||
$scope.error = errorResponse.data.message; | ||
}); | ||
}; | ||
vm.article = article; | ||
vm.authentication = Authentication; | ||
vm.error = null; | ||
vm.form = {}; | ||
vm.remove = remove; | ||
vm.save = save; | ||
|
||
// Remove existing Article | ||
$scope.remove = function (article) { | ||
if (article) { | ||
article.$remove(); | ||
|
||
for (var i in $scope.articles) { | ||
if ($scope.articles[i] === article) { | ||
$scope.articles.splice(i, 1); | ||
} | ||
} | ||
} else { | ||
$scope.article.$remove(function () { | ||
$location.path('articles'); | ||
}); | ||
function remove() { | ||
if (confirm('Are you sure you want to delete?')) { | ||
vm.article.$remove($state.go('articles.list')); | ||
} | ||
}; | ||
|
||
// Update existing Article | ||
$scope.update = function (isValid) { | ||
$scope.error = null; | ||
} | ||
|
||
// Save Article | ||
function save(isValid) { | ||
if (!isValid) { | ||
$scope.$broadcast('show-errors-check-validity', 'articleForm'); | ||
|
||
$scope.$broadcast('show-errors-check-validity', 'vm.form.articleForm'); | ||
return false; | ||
} | ||
|
||
var article = $scope.article; | ||
|
||
article.$update(function () { | ||
$location.path('articles/' + article._id); | ||
}, function (errorResponse) { | ||
$scope.error = errorResponse.data.message; | ||
}); | ||
}; | ||
// TODO: move create/update logic to service | ||
if (vm.article._id) { | ||
vm.article.$update(successCallback, errorCallback); | ||
} else { | ||
vm.article.$save(successCallback, errorCallback); | ||
} | ||
|
||
// Find a list of Articles | ||
$scope.find = function () { | ||
$scope.articles = Articles.query(); | ||
}; | ||
function successCallback(res) { | ||
$state.go('articles.view', { | ||
articleId: res._id | ||
}); | ||
} | ||
|
||
// Find existing Article | ||
$scope.findOne = function () { | ||
$scope.article = Articles.get({ | ||
articleId: $stateParams.articleId | ||
}); | ||
}; | ||
function errorCallback(res) { | ||
vm.error = res.data.message; | ||
} | ||
} | ||
} | ||
]); | ||
})(); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
(function () { | ||
'use strict'; | ||
|
||
angular | ||
.module('articles') | ||
.controller('ArticlesListController', ArticlesListController); | ||
|
||
ArticlesListController.$inject = ['Article']; | ||
|
||
function ArticlesListController(Article) { | ||
var vm = this; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ? |
||
|
||
vm.articles = Article.query(); | ||
} | ||
})(); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,13 @@ | ||
'use strict'; | ||
(function () { | ||
'use strict'; | ||
|
||
//Articles service used for communicating with the articles REST endpoints | ||
angular.module('articles').factory('Articles', ['$resource', | ||
function ($resource) { | ||
angular | ||
.module('articles') | ||
.factory('Articles', Articles); | ||
|
||
Articles.$inject = ['$resource']; | ||
|
||
function Articles($resource) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @trainerbill To follow John Papa to the T, this should be ArticlesService https://github.com/johnpapa/angular-styleguide#naming There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are correct sir. I will update There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Need to update this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This still hasn't been updated. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. update |
||
return $resource('api/articles/:articleId', { | ||
articleId: '@_id' | ||
}, { | ||
|
@@ -11,4 +16,4 @@ angular.module('articles').factory('Articles', ['$resource', | |
} | ||
}); | ||
} | ||
]); | ||
})(); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
(function () { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @rhutchison What's the purpose of this file? We haven't decided on how we're going to move to a client-side model implementation. This isn't needed, since it's just a duplication of article.client.service |
||
'use strict'; | ||
|
||
angular | ||
.module('articles.models') | ||
.factory('Article', Article); | ||
|
||
Article.$inject = ['$resource']; | ||
|
||
function Article($resource) { | ||
return $resource('api/articles/:articleId', { | ||
articleId: '@_id' | ||
}, { | ||
update: { | ||
method: 'PUT' | ||
} | ||
}); | ||
} | ||
})(); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
<section> | ||
<div class="page-header"> | ||
<h1>{{vm.article._id ? 'Edit Article' : 'New Article'}}</h1> | ||
</div> | ||
<div class="col-md-12"> | ||
<form name="vm.form.articleForm" class="form-horizontal" ng-submit="vm.save(vm.form.articleForm.$valid)" novalidate> | ||
<fieldset> | ||
<div class="form-group" show-errors> | ||
<label class="control-label" for="title">Title</label> | ||
<input name="title" type="text" ng-model="vm.article.title" id="title" class="form-control" placeholder="Title" required> | ||
<div ng-messages="vm.form.articleForm.title.$error" role="alert"> | ||
<p class="help-block error-text" ng-message="required">Article title is required.</p> | ||
</div> | ||
</div> | ||
<div class="form-group"> | ||
<label class="control-label" for="content">Content</label> | ||
<textarea name="content" data-ng-model="vm.article.content" id="content" class="form-control" cols="30" rows="10" placeholder="Content"></textarea> | ||
</div> | ||
<div class="form-group"> | ||
<button type="submit" class="btn btn-default">{{vm.article._id ? 'Update' : 'Create'}}</button> | ||
</div> | ||
<div ng-show="vm.error" class="text-danger"> | ||
<strong ng-bind="vm.error"></strong> | ||
</div> | ||
</fieldset> | ||
</form> | ||
</div> | ||
</section> |
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.
Shouldn't this be 'ArticlesCreateController' or something similar since we are doing that above?
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.
Well previously I had them all separate controllers but that got shot down. I can see the need for a create controller if we don't want to pass in a blank article to resolve. Then the ArticlesController would handle edit/view since it needs an article to resolve? @rhutchison