Fix uploadAttachments test

frontend/app/components/api/api-v3/hal-resources/work-package-resource.service.test.ts

@@ -93,15 +93,14 @@ describe('WorkPackageResource service', () => {
       });
     });
 
-    describe('when adding multiple attachments to the work package', () => {
-      var file: any = {};
-      var files: any[] = [file, file];
-      var attachmentsUploadStub;
-      var deferred;
-      var uploadNotificationStub;
+    describe('when a work package is created with attachments and activities', () => {
+      const expectUncachedRequest = href => {
+        $httpBackend
+          .expectGET(href, headers => headers.caching.enabled === false)
+          .respond(200, {_links: {self: {href}}});
+      };
 
       beforeEach(() => {
-        deferred = $q.defer();
         source = {
           _links: {
             attachments: {
@@ -113,121 +112,119 @@ describe('WorkPackageResource service', () => {
           }
         };
         createWorkPackage();
-
-        const uploadResult = {uploads: [deferred.promise], upload: deferred.promise};
-        attachmentsUploadStub = sinon
-          .stub(workPackage.attachments, 'upload')
-          .returns(uploadResult);
-        uploadNotificationStub = sinon.stub(NotificationsService, 'addWorkPackageUpload');
-
-        workPackage.uploadAttachments(files);
       });
 
-      it('should call the upload method of the attachment collection resource', () => {
-        expect(attachmentsUploadStub.calledWith(files)).to.be.true;
-      });
+      describe('when adding multiple attachments to the work package', () => {
+        var file: any = {};
+        var files: any[] = [file, file];
+        var uploadFilesDeferred;
+        var uploadAttachmentsPromise;
+        var attachmentsUploadStub;
+        var uploadNotificationStub;
 
-      it('should add an upload notification', () => {
-        expect(uploadNotificationStub.calledOnce).to.be.true;
-      });
+        beforeEach(() => {
+          uploadFilesDeferred = $q.defer();
+          const uploadResult = {uploads: [uploadFilesDeferred.promise], upload: uploadFilesDeferred.promise};
+          attachmentsUploadStub = sinon.stub(workPackage.attachments, 'upload').returns(uploadResult);
+          uploadNotificationStub = sinon.stub(NotificationsService, 'addWorkPackageUpload');
 
-      describe('when the upload fails', () => {
-        var notificationStub;
-        var error = 'err';
+          uploadAttachmentsPromise = workPackage.uploadAttachments(files);
+        });
 
-        beforeEach(() => {
-          deferred.reject(error);
-          notificationStub = sinon.stub(wpNotificationsService, 'handleErrorResponse');
-          $rootScope.$apply();
+        it('should call the upload method of the attachment collection resource', () => {
+          expect(attachmentsUploadStub.calledWith(files)).to.be.true;
         });
 
-        it('should call the error response notification', () => {
-          expect(notificationStub.calledWith(error, workPackage)).to.be.true;
+        it('should add an upload notification', () => {
+          expect(uploadNotificationStub.calledOnce).to.be.true;
         });
-      });
 
-      describe('when the upload succeeds', () => {
-        var removeStub;
+        describe('when the upload fails', () => {
+          var notificationStub;
+          var error = 'err';
 
-        beforeEach(() => {
-          deferred.resolve();
-          removeStub = sinon.stub(NotificationsService, 'remove');
-          $rootScope.$apply();
+          beforeEach(() => {
+            uploadFilesDeferred.reject(error);
+            notificationStub = sinon.stub(wpNotificationsService, 'handleErrorResponse');
+            $rootScope.$apply();
+          });
+
+          it('should call the error response notification', () => {
+            expect(notificationStub.calledWith(error, workPackage)).to.be.true;
+          });
         });
 
-        it('should remove the upload notification', angular.mock.inject($timeout => {
-          $timeout.flush();
-          expect(removeStub.calledOnce).to.be.true;
-        }));
+        describe('when the upload succeeds', () => {
+          var removeStub;
 
-        it('should return an attachment collection resource promise', () => {
-          expect(deferred.promise).to.eventually.have.property('$href', 'attachments');
-          $rootScope.$apply();
-        });
-      });
-    });
+          beforeEach(() => {
+            uploadFilesDeferred.resolve();
+            removeStub = sinon.stub(NotificationsService, 'remove');
 
-    describe('when updating multiple linked resources', () => {
-      var updateWorkPackageStub: SinonStub;
-      var promise: any;
+            expectUncachedRequest('activities');
+            expectUncachedRequest('attachments');
+            $httpBackend.flush();
+            $rootScope.$apply();
+          });
 
-      const testWpCacheUpdateWith = (prepare, ...urls) => {
-        beforeEach(() => {
-          prepare();
+          it('should remove the upload notification', angular.mock.inject($timeout => {
+            $timeout.flush();
+            expect(removeStub.calledOnce).to.be.true;
+          }));
 
-          urls.forEach(href => {
-            $httpBackend
-              .expectGET(href, headers => headers.caching.enabled === false)
-              .respond(200, {_links: {self: {href}}});
+          it('should return an attachment collection resource promise', () => {
+            expect(uploadAttachmentsPromise).to.eventually.have.property('$href', 'attachments');
+            $rootScope.$apply();
           });
-          $httpBackend.flush();
         });
+      });
 
-        it('should update the work package cache', () => {
-          expect(updateWorkPackageStub.calledWith(workPackage)).to.be.true;
-        });
-      };
+      describe('when updating multiple linked resources', () => {
+        var updateWorkPackageStub: SinonStub;
+        var promise: any;
 
-      const testLinkedResource = href => {
-        it('should return a promise that returns the ' + href, () => {
-          expect(promise).to.eventually.have.property('$href', href);
-        });
-      };
+        const testWpCacheUpdateWith = (prepare, ...urls) => {
+          beforeEach(() => {
+            prepare();
+            urls.forEach(expectUncachedRequest);
+            $httpBackend.flush();
+          });
 
-      beforeEach(() => {
-        source = {
-          _links: {
-            activities: {
-              href: 'activities'
-            },
-            attachments: {
-              href: 'attachments'
-            }
-          }
+          it('should update the work package cache', () => {
+            expect(updateWorkPackageStub.calledWith(workPackage)).to.be.true;
+          });
         };
-        updateWorkPackageStub = sinon.stub(wpCacheService, 'updateWorkPackage');
-        createWorkPackage();
-      });
 
-      afterEach(() => {
-        $rootScope.$apply();
-        updateWorkPackageStub.restore();
-      });
+        const testLinkedResource = href => {
+          it('should return a promise that returns the ' + href, () => {
+            expect(promise).to.eventually.have.property('$href', href);
+          });
+        };
 
-      describe('when updating the activities', () => {
-        testWpCacheUpdateWith(() => {
-          promise = workPackage.updateActivities();
-        }, 'activities');
+        beforeEach(() => {
+          updateWorkPackageStub = sinon.stub(wpCacheService, 'updateWorkPackage');
+        });
 
-        testLinkedResource('activities');
-      });
+        afterEach(() => {
+          $rootScope.$apply();
+          updateWorkPackageStub.restore();
+        });
+
+        describe('when updating the activities', () => {
+          testWpCacheUpdateWith(() => {
+            promise = workPackage.updateActivities();
+          }, 'activities');
 
-      describe('when updating the attachments', () => {
-        testWpCacheUpdateWith(() => {
-          promise = workPackage.updateAttachments();
-        }, 'activities', 'attachments');
+          testLinkedResource('activities');
+        });
+
+        describe('when updating the attachments', () => {
+          testWpCacheUpdateWith(() => {
+            promise = workPackage.updateAttachments();
+          }, 'activities', 'attachments');
 
-        testLinkedResource('attachments');
+          testLinkedResource('attachments');
+        });
       });
     });
   });

frontend/app/components/api/api-v3/hal-resources/work-package-resource.service.ts

@@ -196,7 +196,7 @@ export class WorkPackageResource extends HalResource {
     const message = I18n.t('js.label_upload_notification', this);
     const notification = NotificationsService.addWorkPackageUpload(message, uploads);
 
-    upload
+    return upload
       .then(() => {
         $timeout(() => {
           NotificationsService.remove(notification);
@@ -207,8 +207,6 @@ export class WorkPackageResource extends HalResource {
       .catch(error => {
         wpNotificationsService.handleErrorResponse(error, this);
       });
-
-    return upload;
   }
 
   public requiredValueFor(fieldName): boolean {