Merge remote-tracking branch 'remotes/origin/master' into feature/aut…

…horization * remotes/origin/master: Applied fixes from StyleCI Tidied user credentials updating now _user_profile has gone from usercontroller Added test for saving user without any changes Refactored all private keys with underscored keys to have double underscore, extended getAttributes function to ignore these keys Applied fixes from StyleCI Fixes to make the user model and services handle nested entities correctly. Abstracted common api service functionality to an AbstractApiService to cut down on the code duplication # Conflicts: # api/app/Http/Controllers/UserController.php
spira · Sep 22, 2015 · d11fd98 · d11fd98
2 parents e840ad0 + 6a3441a
commit d11fd98
Show file tree

Hide file tree

Showing 23 changed files with 255 additions and 205 deletions.
diff --git a/api/app/Http/Controllers/UserController.php b/api/app/Http/Controllers/UserController.php
@@ -62,9 +62,6 @@ public function __construct(
      */
     public function putOne(Request $request, $id)
     {
-        // Extract the credentials
-        $credential = $request->input('_user_credential', []);
-
         // Set new users to guest
         $request->merge(['user_type' => 'guest']);
 
@@ -81,8 +78,10 @@ public function putOne(Request $request, $id)
         $model->save();
 
         // Finally create the credentials
-        $this->validateRequest($credential, UserCredential::getValidationRules());
-        $model->setCredential(new UserCredential($credential));
+        if ($credential = $request->input('_user_credential', null)) {
+            $this->validateRequest($credential, UserCredential::getValidationRules());
+            $model->setCredential(new UserCredential($credential));
+        }
 
         return $this->getResponse()
             ->transformer($this->getTransformer())
@@ -137,16 +136,15 @@ public function patchOne(Request $request, $id)
         }
 
         // Extract the credentials and update if necessary
-        $credentialUpdateDetails = $request->input('_user_credential', []);
-        if (! empty($credentialUpdateDetails)) {
+        $credentialUpdateDetails = $request->input('_user_credential');
+        if ($credentialUpdateDetails) {
             // Invalidate token for the user when user changes their password
             if ($request->user()->user_id == $model->user_id) {
                 $this->auth->logout();
             }
 
-            $credentials = UserCredential::findOrNew($id);
             /* @var UserCredential $credentials */
-            $credentials->fill($credentialUpdateDetails);
+            $credentials = UserCredential::findOrNew($id)->fill($credentialUpdateDetails);
             $model->setCredential($credentials);
         }
 
@@ -196,37 +194,10 @@ public function unlinkSocialLogin($id, $provider)
         $user = User::find($id);
         $this->auth->login($user, true);
 
-        return $this->getResponse()->header('Authorization-Update', $this->auth->generateToken($user))->noContent();
-    }
-
-    /**
-     * Get full user details.
-     *
-     * @param Request $request
-     * @param string $id
-     * @return \Spira\Responder\Response\ApiResponse
-     */
-    public function getOne(Request $request, $id)
-    {
-        /** @var User $user */
-        $user = User::find($id);
-        $this->authorize($user);
-        $userData = $this->transformer->transformItem($user);
-
-        if (is_null($user->userCredential)) {
-            $userData['_user_credential'] = false;
-        } else {
-            $userData['_user_credential'] = $user->userCredential->toArray();
-        }
-
-        if (is_null($user->socialLogins)) {
-            $userData['_social_logins'] = false;
-        } else {
-            $userData['_social_logins'] = $user->socialLogins->toArray();
-        }
 
         return $this->getResponse()
-            ->transformer($this->getTransformer())
-            ->item($userData);
+            ->header('Authorization-Update', $this->auth->generateToken($user))
+            ->noContent();
     }
+
 }
diff --git a/app/src/app/admin/articles/listing/listing.spec.ts b/app/src/app/admin/articles/listing/listing.spec.ts
@@ -22,7 +22,7 @@ namespace app.admin.articles.listing {
                 articleService = _articleService_;
 
                 // Setup articlesPaginator before injection
-                articlesPaginator = articleService.getArticlesPaginator().setCount(10);
+                articlesPaginator = articleService.getPaginator().setCount(10);
 
                 ArticlesListingController = $controller(app.admin.articles.listing.namespace + '.controller', {
                     articlesPaginator: articlesPaginator,

diff --git a/app/src/app/admin/articles/listing/listing.ts b/app/src/app/admin/articles/listing/listing.ts
@@ -26,7 +26,7 @@ namespace app.admin.articles.listing {
                 },
                 resolve: /*@ngInject*/{
                     articlesPaginator: (articleService:common.services.article.ArticleService) => {
-                        return articleService.getArticlesPaginator().setCount(12);
+                        return articleService.getPaginator().setCount(12);
                     },
                     initArticles: (articlesPaginator:common.services.pagination.Paginator, $stateParams:IArticlesListingStateParams) => {
                         return articlesPaginator.getPage($stateParams.page);

diff --git a/app/src/app/admin/media/media.spec.ts b/app/src/app/admin/media/media.spec.ts
@@ -24,7 +24,7 @@ namespace app.admin.media {
                 imageService = _imageService_;
 
                 // Setup imagesPaginator before injection
-                imagesPaginator = imageService.getImagesPaginator().setCount(12);
+                imagesPaginator = imageService.getPaginator().setCount(12);
                 imagesPaginator.entityCountTotal = 50;
                 images = common.models.ImageMock.collection(12);
 

diff --git a/app/src/app/admin/media/media.ts b/app/src/app/admin/media/media.ts
@@ -29,7 +29,7 @@ namespace app.admin.media {
                 resolve: /*@ngInject*/{
                     perPage: () => 12,
                     imagesPaginator: (imageService:common.services.image.ImageService, perPage:number) => {
-                        return imageService.getImagesPaginator().setCount(perPage);
+                        return imageService.getPaginator().setCount(perPage);
                     },
                     initialImages: (imagesPaginator:common.services.pagination.Paginator, $stateParams:IMediaStateParams):ng.IPromise<common.models.Image[]> => {
                         return imagesPaginator.getPage($stateParams.page);

diff --git a/app/src/app/guest/articles/articles.ts b/app/src/app/guest/articles/articles.ts
@@ -18,7 +18,7 @@ namespace app.guest.articles {
                 },
                 resolve: /*@ngInject*/{
                     articlesPaginator: (articleService:common.services.article.ArticleService) => {
-                        return articleService.getArticlesPaginator().setCount(5);
+                        return articleService.getPaginator().setCount(5);
                     },
                     initialArticles: (articlesPaginator:common.services.pagination.Paginator) => {
                         return articlesPaginator.getNext();

diff --git a/app/src/app/user/profile/profile.ts b/app/src/app/user/profile/profile.ts
@@ -67,9 +67,9 @@ namespace app.user.profile {
                     },
                     fullUserInfo:(
                         userService:common.services.user.UserService,
-                        ngJwtAuthService:NgJwtAuth.NgJwtAuthService
+                        user:common.models.User //inherited from parent state
                     ) => {
-                        return userService.getUser(<common.models.User>ngJwtAuthService.getUser())
+                        return userService.getUser(user, ['userCredential', 'userProfile', 'socialLogins'])
                     },
                     genderOptions:() => {
                         return common.models.UserProfile.genderOptions;

diff --git a/app/src/common/decorators/changeAwareDecorator.spec.ts b/app/src/common/decorators/changeAwareDecorator.spec.ts
@@ -5,7 +5,7 @@ namespace common.decorators {
     @changeAware
     class TestModel extends common.models.AbstractModel {
 
-        protected _nestedEntityMap:common.models.INestedEntityMap = {
+        protected __nestedEntityMap:common.models.INestedEntityMap = {
             _nestedCollection: NestedData,
             _nestedEntity: NestedData,
         };

diff --git a/app/src/common/models/abstractModel.spec.ts b/app/src/common/models/abstractModel.spec.ts
@@ -5,12 +5,13 @@ namespace common.models {
 
     class TestModel extends AbstractModel {
 
-        protected _nestedEntityMap:INestedEntityMap = {
+        protected __nestedEntityMap:INestedEntityMap = {
             hasOne: TestChildModel,
             hasMany: TestChildModel,
             hydrate: this.hydrateFunction
         };
 
+        public foo:string = undefined;
         public _hasOne:TestChildModel;
         public _hasMany:TestChildModel[];
         public _hydrate:TestChildModel[];
@@ -43,7 +44,7 @@ namespace common.models {
         it('should instantiate nested entities', () => {
 
             let model = new TestModel({
-                '_hasOne' : {},
+                _hasOne : {},
             });
 
             expect(model._hasOne).to.be.instanceOf(TestChildModel);
@@ -53,7 +54,7 @@ namespace common.models {
         it('should instantiate nested collections (class)', () => {
 
             let model = new TestModel({
-                '_hasMany' : [{}]
+                _hasMany : [{}]
             });
 
             expect(model._hasMany[0]).to.be.instanceOf(TestChildModel);
@@ -63,7 +64,7 @@ namespace common.models {
         it('should instantiate nested collections (function, exists)', () => {
 
             let model = new TestModel({
-                '_hydrate' : ['foobar']
+                _hydrate : ['foobar']
             }, true);
 
             expect(model._hydrate).to.deep.equal(['bar']);
@@ -73,13 +74,42 @@ namespace common.models {
         it('should instantiate nested collections (function, non-existant)', () => {
 
             let model = new TestModel({
-                '_hydrate' : ['foobar']
+                _hydrate : ['foobar']
             }, false);
 
             expect(model._hydrate).to.deep.equal(['foobar']);
 
         });
 
+        it('should be able to retrieve a model\'s attributes without nested keys', () => {
+
+            let model = new TestModel({
+                foo: 'bar',
+                _hydrate : ['foobar']
+            }, false);
+
+            expect(model.getAttributes()).to.deep.equal({
+                foo:'bar'
+            });
+
+        });
+
+        it('should be able to retrieve a model\'s attributes with nested keys', () => {
+
+            let model = new TestModel({
+                foo: 'bar',
+                _hydrate : ['foobar']
+            }, false);
+
+            expect(model.getAttributes(true)).to.deep.equal({
+                foo: 'bar',
+                _hydrate : ['foobar'],
+                _hasOne: null,
+                _hasMany: null,
+            });
+
+        });
+
 
         it('should be able to check if a model exists on the remote api', () => {
 

diff --git a/app/src/common/models/abstractModel.ts b/app/src/common/models/abstractModel.ts
@@ -25,13 +25,13 @@ namespace common.models {
 
     export abstract class AbstractModel implements IModel {
 
-        protected _nestedEntityMap:INestedEntityMap;
-        private _exists:boolean;
+        protected __nestedEntityMap:INestedEntityMap;
+        private __exists:boolean;
 
         constructor(data?:any, exists:boolean = false) {
             this.hydrate(data, exists);
 
-            Object.defineProperty(this, "_exists", {
+            Object.defineProperty(this, "__exists", {
                 enumerable: false,
                 writable: true,
                 value: exists,
@@ -47,7 +47,7 @@ namespace common.models {
             if (_.isObject(data)) {
                 _.assign(this, data);
 
-                if (_.size(this._nestedEntityMap) > 1) {
+                if (_.size(this.__nestedEntityMap) > 1) {
                     this.hydrateNested(data, exists);
                 }
             }
@@ -74,7 +74,7 @@ namespace common.models {
          */
         protected hydrateNested(data:any, exists:boolean){
 
-            _.forIn(this._nestedEntityMap, (nestedObject:IModelClass|IHydrateFunction, nestedKey:string) => {
+            _.forIn(this.__nestedEntityMap, (nestedObject:IModelClass|IHydrateFunction, nestedKey:string) => {
 
                 //if the nested map is not defined with a leading _ prepend one
                 if (!_.startsWith(nestedKey, '_')){
@@ -124,13 +124,17 @@ namespace common.models {
          */
         public getAttributes(includeUnderscoredKeys:boolean = false):Object{
 
-            let attributes = _.clone(this);
+            let allAttributes = _.clone(this);
+
+            let publicAttributes = _.omit(allAttributes, (value, key) => {
+                return _.startsWith(key, '__');
+            });
 
             if (includeUnderscoredKeys){
-                return attributes;
+                return publicAttributes;
             }
 
-            return _.omit(attributes, (value, key) => {
+            return _.omit(publicAttributes, (value, key) => {
                 return _.startsWith(key, '_');
             });
 
@@ -141,15 +145,15 @@ namespace common.models {
          * @returns {boolean}
          */
         public exists():boolean{
-            return this._exists;
+            return this.__exists;
         }
 
         /**
          * Set if the model exists
          * @param exists
          */
         public setExists(exists:boolean):void{
-            this._exists = exists;
+            this.__exists = exists;
         }
 
         /**

diff --git a/app/src/common/models/article/articleModel.ts b/app/src/common/models/article/articleModel.ts
@@ -3,7 +3,7 @@ namespace common.models {
     @common.decorators.changeAware
     export class Article extends AbstractModel {
 
-        protected _nestedEntityMap:INestedEntityMap = {
+        protected __nestedEntityMap:INestedEntityMap = {
             tags: Tag,
             author: User,
             articleMetas: this.hydrateMetaCollectionFromTemplate

diff --git a/app/src/common/models/user/userModel.ts b/app/src/common/models/user/userModel.ts
@@ -3,6 +3,12 @@ namespace common.models {
     @common.decorators.changeAware
     export class User extends AbstractModel implements global.IUserData {
 
+        protected __nestedEntityMap:INestedEntityMap = {
+            userProfile: UserProfile,
+            socialLogins: UserSocialLogin,
+            userCredential: UserCredential,
+        };
+
         public static adminType = 'admin';
         public static guestType = 'guest';
         public static userTypes:string[] = [User.adminType, User.guestType];

diff --git a/app/src/common/services/abstractApiService.ts b/app/src/common/services/abstractApiService.ts
@@ -0,0 +1,56 @@
+namespace common.services {
+
+    export abstract class AbstractApiService {
+
+
+        protected cachedPaginator:common.services.pagination.Paginator;
+
+        constructor(protected ngRestAdapter:NgRestAdapter.INgRestAdapterService,
+                    protected paginationService:common.services.pagination.PaginationService,
+                    protected $q:ng.IQService) {
+        }
+
+
+        protected abstract modelFactory(data:any, exists?:boolean):common.models.IModel;
+        protected abstract apiEndpoint():string;
+
+        /**
+         * Get the paginator
+         * @returns {Paginator}
+         */
+        public getPaginator():common.services.pagination.Paginator {
+
+            //cache the paginator so subsequent requests can be collection length-aware
+            if (!this.cachedPaginator){
+                this.cachedPaginator = this.paginationService
+                    .getPaginatorInstance(this.apiEndpoint())
+                    .setModelFactory(this.modelFactory);
+            }
+
+            return this.cachedPaginator;
+        }
+
+        /**
+         * Get model response given an identifier (uuid or permalink)
+         * @param identifier
+         * @param withNested
+         * @returns {ng.IHttpPromise<any>}
+         */
+        protected getModel(identifier:string, withNested:string[] = null):ng.IPromise<ng.IHttpPromiseCallbackArg<any>> {
+
+            return this.ngRestAdapter.get(this.apiEndpoint()+'/'+identifier, {
+                'With-Nested' : () => {
+                    if (!withNested){
+                        return null;
+                    }
+                    return withNested.join(', ');
+                }
+            });
+        }
+
+
+    }
+}
+
+
+
diff --git a/app/src/common/services/article/articleService.spec.ts b/app/src/common/services/article/articleService.spec.ts
@@ -108,7 +108,7 @@
 
                 $httpBackend.expectGET('/api/articles').respond(_.take(articles, 10));
 
-                let articlePaginator = articleService.getArticlesPaginator();
+                let articlePaginator = articleService.getPaginator();
 
                 let firstSet = articlePaginator.getNext(10);