From d91116e6378b6cdb27f2a5be23f52ca6b7745066 Mon Sep 17 00:00:00 2001 From: Nick Litwin Date: Wed, 3 Feb 2016 17:51:52 -0800 Subject: [PATCH 1/2] Fix windows and IE bugs --- .../tc-file-input/tc-file-input.directive.js | 68 +++++++++++-------- .../submit-develop-files.jade | 2 +- 2 files changed, 40 insertions(+), 30 deletions(-) diff --git a/app/directives/tc-file-input/tc-file-input.directive.js b/app/directives/tc-file-input/tc-file-input.directive.js index 6fdaf6e9c..4324733d0 100644 --- a/app/directives/tc-file-input/tc-file-input.directive.js +++ b/app/directives/tc-file-input/tc-file-input.directive.js @@ -4,9 +4,9 @@ import _ from 'lodash' (function() { 'use strict' - angular.module('tcUIComponents').directive('tcFileInput', tcFileInput) + angular.module('tcUIComponents').directive('tcFileInput', ['$timeout', tcFileInput]) - function tcFileInput() { + function tcFileInput($timeout) { return { restrict: 'E', require: '^form', @@ -26,6 +26,13 @@ import _ from 'lodash' scope.selectFile = selectFile var fileTypes = scope.fileType.split(',') + // Add extra checks for Windows zip file types + var hasZip = _.some(fileTypes, _.matches('zip')) + if (hasZip) { + fileTypes = angular.copy(fileTypes) + fileTypes.push('x-zip', 'x-zip-compressed') + } + // fieldId is not set on element at this point, so grabbing with class .none // which exists on the element right away var fileInput = $(element[0]).find('.none') @@ -46,33 +53,36 @@ import _ from 'lodash' var selectedFileType = file.type.slice(file.type.lastIndexOf('/') + 1) var isAllowedFileFormat = _.some(fileTypes, _.matches(selectedFileType)) - scope.$apply(function(){ - // Set the file name as the value of the disabled input - fileNameInput[0].value = file.name - var clickedFileInput = formController[attrs.fieldId] - - if (!isAllowedFileFormat) { - // Manually setting is required since Angular doesn't support file inputs - clickedFileInput.$setTouched() - clickedFileInput.$setValidity('required', false) - - } else { - clickedFileInput.$setValidity('required', true) - } - - if (!isAllowedFileSize) { - // Manually setting is required since Angular doesn't support file inputs - clickedFileInput.$setTouched() - clickedFileInput.$setValidity('filesize', false) - - } else { - clickedFileInput.$setValidity('filesize', true) - } - - if (isAllowedFileFormat && isAllowedFileSize) { - // Pass file object up through callback into controller - scope.setFileReference({file: file, fieldId: scope.fieldId}) - } + // Timeout needed for fixing IE bug ($apply already in progress) + $timeout(function() { + scope.$apply(function(){ + // Set the file name as the value of the disabled input + fileNameInput[0].value = file.name + var clickedFileInput = formController[attrs.fieldId] + + if (!isAllowedFileFormat) { + // Manually setting is required since Angular doesn't support file inputs + clickedFileInput.$setTouched() + clickedFileInput.$setValidity('required', false) + + } else { + clickedFileInput.$setValidity('required', true) + } + + if (!isAllowedFileSize) { + // Manually setting is required since Angular doesn't support file inputs + clickedFileInput.$setTouched() + clickedFileInput.$setValidity('filesize', false) + + } else { + clickedFileInput.$setValidity('filesize', true) + } + + if (isAllowedFileFormat && isAllowedFileSize) { + // Pass file object up through callback into controller + scope.setFileReference({file: file, fieldId: scope.fieldId}) + } + }) }) }) diff --git a/app/submissions/submit-develop-files/submit-develop-files.jade b/app/submissions/submit-develop-files/submit-develop-files.jade index 9697b045d..6ebe307af 100644 --- a/app/submissions/submit-develop-files/submit-develop-files.jade +++ b/app/submissions/submit-develop-files/submit-develop-files.jade @@ -31,7 +31,7 @@ label-text="Preview Image", field-id="DESIGN_COVER", button-text="Add File", - file-type="jpg,jpeg,png" + file-type="zip" placeholder="Image file as .jpg or .png", mandatory="true", set-file-reference="vm.setFileReference(file, fieldId)", From de82801ef603f4acd9c828b9f2710b7fdc7a711e Mon Sep 17 00:00:00 2001 From: Nick Litwin Date: Wed, 3 Feb 2016 19:00:06 -0800 Subject: [PATCH 2/2] Add tests for windows and ie changes --- .../tc-file-input/tc-file-input.directive.js | 1 + .../tc-file-input/tc-file-input.spec.js | 188 +++++++------ .../submit-design-files.spec.js | 246 +++++++++--------- 3 files changed, 237 insertions(+), 198 deletions(-) diff --git a/app/directives/tc-file-input/tc-file-input.directive.js b/app/directives/tc-file-input/tc-file-input.directive.js index 4324733d0..94d68b347 100644 --- a/app/directives/tc-file-input/tc-file-input.directive.js +++ b/app/directives/tc-file-input/tc-file-input.directive.js @@ -28,6 +28,7 @@ import _ from 'lodash' // Add extra checks for Windows zip file types var hasZip = _.some(fileTypes, _.matches('zip')) + if (hasZip) { fileTypes = angular.copy(fileTypes) fileTypes.push('x-zip', 'x-zip-compressed') diff --git a/app/directives/tc-file-input/tc-file-input.spec.js b/app/directives/tc-file-input/tc-file-input.spec.js index ef1917cc8..b63bcede7 100644 --- a/app/directives/tc-file-input/tc-file-input.spec.js +++ b/app/directives/tc-file-input/tc-file-input.spec.js @@ -1,148 +1,186 @@ +import angular from 'angular' + /* jshint -W117, -W030 */ describe('Topcoder File Input Directive', function() { - var scope, element, isolateScope, fileInput; + var scope, element, isolateScope, fileInput beforeEach(function() { - bard.appModule('topcoder'); - bard.inject(this, '$compile', '$rootScope'); - scope = $rootScope.$new(); + bard.appModule('topcoder') + bard.inject(this, '$compile', '$rootScope', '$timeout') + scope = $rootScope.$new() var html = '' + '
' + '' + - ''; - var form = angular.element(html); - element = form.find('tc-file-input'); - var formElement = $compile(form)(scope); - scope.$digest(); + '' + var form = angular.element(html) + element = form.find('tc-file-input') + var formElement = $compile(form)(scope) + scope.$digest() - isolateScope = element.isolateScope(); - }); + isolateScope = element.isolateScope() + }) beforeEach(function() { - fileInput = $(element).find('.none')[0]; - }); + fileInput = $(element).find('.none')[0] + }) afterEach(function() { - scope.$destroy(); - fileInput = undefined; - }); + scope.$destroy() + fileInput = undefined + }) - bard.verifyNoOutstandingHttpRequests(); + bard.verifyNoOutstandingHttpRequests() describe('selectFile', function() { it('triggers a click on the file input', function() { - var mockClick = sinon.spy(fileInput, 'click'); + var mockClick = sinon.spy(fileInput, 'click') - isolateScope.selectFile(); - scope.$digest(); + isolateScope.selectFile() + scope.$digest() - expect(mockClick).calledOnce; - }); - }); + expect(mockClick).calledOnce + }) + }) describe('a change event on the file input', function() { - var fileNameInput, fileList, mockSetFileReference; + var fileNameInput, fileList, mockSetFileReference beforeEach(function() { - fileNameInput = $(element).find('input[type=text]')[0]; + fileNameInput = $(element).find('input[type=text]')[0] fileList = { 0: { - name: 'test.png', + name: 'test.zip', size: 50, - type: 'image/png' + type: 'application/zip' }, length: 1, - item: function (index) { return file; } - }; + item: function (index) { return file } + } - mockSetFileReference = sinon.spy(isolateScope, 'setFileReference'); - }); + mockSetFileReference = sinon.spy(isolateScope, 'setFileReference') + }) afterEach(function() { - fileNameInput = undefined; - fileList = undefined; - mockSetFileReference = undefined; - }); + fileNameInput = undefined + fileList = undefined + mockSetFileReference = undefined + }) it('sets the value of the fileNameInput with the name of the file', function() { $(fileInput).triggerHandler({ type: 'change', target: { files: fileList } - }); + }) + + $timeout.flush() - expect(fileNameInput.value).to.equal('test.png'); - }); + expect(fileNameInput.value).to.equal('test.zip') + }) describe('with a valid file', function() { beforeEach(function() { $(fileInput).triggerHandler({ type: 'change', target: { files: fileList } - }); - }); + }) + $timeout.flush() + }) it('calls setFileReference', function() { - expect(mockSetFileReference).calledOnce; - }); + expect(mockSetFileReference).calledOnce + }) it('has ng-valid-filesize class', function() { - expect($(fileInput).hasClass('ng-valid-filesize')).to.be.true; - }); + expect($(fileInput).hasClass('ng-valid-filesize')).to.be.true + }) it('has ng-valid-required class', function() { - expect($(fileInput).hasClass('ng-valid-required')).to.be.true; - }); - }); + expect($(fileInput).hasClass('ng-valid-required')).to.be.true + }) - describe('with a file that\'s greater than 500MB', function() { - beforeEach(function() { - fileList[0].size = 500000001; + it('works with Windows file type application/x-zip', function(){ + fileList[0].type = 'application/x-zip' $(fileInput).triggerHandler({ type: 'change', target: { files: fileList } - }); - }); + }) - it('does not call setFileReference', function() { - expect(mockSetFileReference).not.calledOnce; - }); + $timeout.flush() - it('has ng-touched and ng-invalid-filesize classes', function() { - expect($(fileInput).hasClass('ng-invalid-filesize')).to.be.true; - expect($(fileInput).hasClass('ng-touched')).to.be.true; - }); - }); + expect(mockSetFileReference).called + expect($(fileInput).hasClass('ng-valid-filesize')).to.be.true + expect($(fileInput).hasClass('ng-valid-required')).to.be.true + }) + + it('works with Windows file type application/x-zip-compressed', function(){ + fileList[0].type = 'application/x-zip-compressed' + + $(fileInput).triggerHandler({ + type: 'change', + target: { files: fileList } + }) + + $timeout.flush() + + expect(mockSetFileReference).called + expect($(fileInput).hasClass('ng-valid-filesize')).to.be.true + expect($(fileInput).hasClass('ng-valid-required')).to.be.true + }) + }) describe('with a file type that\'s not in the list of fileTypes given to the directive', function() { beforeEach(function() { - fileList[0].type = 'application/zip'; + fileList[0].type = 'image/png' $(fileInput).triggerHandler({ type: 'change', target: { files: fileList } - }); - }); + }) + + $timeout.flush() + }) it('does not call setFileReference', function() { - expect(mockSetFileReference).not.calledOnce; - }); + expect(mockSetFileReference).not.calledOnce + }) it('has ng-touched and ng-invalid-required classes', function() { - expect($(fileInput).hasClass('ng-invalid-required')).to.be.true; - expect($(fileInput).hasClass('ng-touched')).to.be.true; - }); - }); + expect($(fileInput).hasClass('ng-invalid-required')).to.be.true + expect($(fileInput).hasClass('ng-touched')).to.be.true + }) + }) + + describe('with a file that\'s greater than 500MB', function() { + beforeEach(function() { + fileList[0].size = 500000001 + + $(fileInput).triggerHandler({ + type: 'change', + target: { files: fileList } + }) + + $timeout.flush() + }) - }); -}); + it('does not call setFileReference', function() { + expect(mockSetFileReference).not.calledOnce + }) + + it('has ng-touched and ng-invalid-filesize classes', function() { + expect($(fileInput).hasClass('ng-invalid-filesize')).to.be.true + expect($(fileInput).hasClass('ng-touched')).to.be.true + }) + }) + }) +}) diff --git a/app/submissions/submit-design-files/submit-design-files.spec.js b/app/submissions/submit-design-files/submit-design-files.spec.js index 13898bd86..1dff69b94 100644 --- a/app/submissions/submit-design-files/submit-design-files.spec.js +++ b/app/submissions/submit-design-files/submit-design-files.spec.js @@ -1,6 +1,6 @@ /* jshint -W117, -W030 */ describe('Submit Design Files Controller', function() { - var controller, vm, scope; + var controller, vm, scope var mockChallenge = { challenge: { @@ -8,34 +8,34 @@ describe('Submit Design Files Controller', function() { track: 'DESIGN', id: 30049240 } - }; + } var userService = { getUserIdentity: function() { return { userId: 123456 - }; + } } - }; + } var submissionsService = { getPresignedURL: function() {} - }; + } var mockWindow = { location: { - reload: function(val) { return val; } + reload: function(val) { return val } } - }; + } beforeEach(function() { - bard.appModule('tc.submissions'); - bard.inject(this, '$controller', '$rootScope'); + bard.appModule('tc.submissions') + bard.inject(this, '$controller', '$rootScope') - scope = $rootScope.$new(); - }); + scope = $rootScope.$new() + }) - bard.verifyNoOutstandingHttpRequests(); + bard.verifyNoOutstandingHttpRequests() beforeEach(function() { controller = $controller('SubmitDesignFilesController', { @@ -44,13 +44,13 @@ describe('Submit Design Files Controller', function() { challengeToSubmitTo: mockChallenge, SubmissionsService: submissionsService, $window: mockWindow - }); - vm = controller; - }); + }) + vm = controller + }) it('exists', function() { - expect(vm).to.exist; - }); + expect(vm).to.exist + }) it('sets the right track for the method', function() { controller = $controller('SubmitDesignFilesController', { @@ -65,136 +65,136 @@ describe('Submit Design Files Controller', function() { }, SubmissionsService: submissionsService, $window: mockWindow - }); - vm = controller; - scope.$digest(); + }) + vm = controller + scope.$digest() - expect(vm.submissionsBody.data.method).to.equal('DEVELOP_CHALLENGE_ZIP_FILE'); - }); + expect(vm.submissionsBody.data.method).to.equal('DEVELOP_CHALLENGE_ZIP_FILE') + }) describe('setRankTo1', function() { it('returns 1 if the input is blank', function() { - expect(vm.setRankTo1('')).to.equal(1); - }); + expect(vm.setRankTo1('')).to.equal(1) + }) it('returns the input value if not blank', function() { - var inputText = 'sample input text'; - var result = vm.setRankTo1(inputText); + var inputText = 'sample input text' + var result = vm.setRankTo1(inputText) - expect(result).to.equal(inputText); - }); - }); + expect(result).to.equal(inputText) + }) + }) describe('setFileReference', function() { - var file, fieldId; + var file, fieldId beforeEach(function() { file = { name: 'Dashboard 2.png', size: 575548, type: 'image/png' - }; - fieldId = 'DESIGN_COVER'; + } + fieldId = 'DESIGN_COVER' - vm.setFileReference(file, fieldId); - scope.$digest(); - }); + vm.setFileReference(file, fieldId) + scope.$digest() + }) afterEach(function() { - file = undefined; - fieldId = undefined; - }); + file = undefined + fieldId = undefined + }) it('adds a file object to the submissions body', function() { - expect(vm.submissionsBody.data.files).to.have.length(1); - }); + expect(vm.submissionsBody.data.files).to.have.length(1) + }) it('replaces a file object with a new one if it has the same fieldId', function() { - expect(vm.submissionsBody.data.files).to.have.length(1); + expect(vm.submissionsBody.data.files).to.have.length(1) var newFile = { name: 'different_image.png', size: 4321, type: 'image/png' - }; + } - vm.setFileReference(newFile, fieldId); - scope.$digest(); + vm.setFileReference(newFile, fieldId) + scope.$digest() - expect(vm.submissionsBody.data.files).to.have.length(1); - expect(vm.submissionsBody.data.files[0].name).to.equal('different_image.png'); - }); + expect(vm.submissionsBody.data.files).to.have.length(1) + expect(vm.submissionsBody.data.files[0].name).to.equal('different_image.png') + }) it('sets the correct mediaTypes on the fileObject', function() { - expect(vm.submissionsBody.data.files[0].mediaType).to.equal('image/png'); + expect(vm.submissionsBody.data.files[0].mediaType).to.equal('image/png') var newFile = { name: 'submission.zip', size: 43121, type: 'application/zip' - }; - var newFieldId = 'SUBMISSION_ZIP'; + } + var newFieldId = 'SUBMISSION_ZIP' - vm.setFileReference(newFile, newFieldId); - scope.$digest(); + vm.setFileReference(newFile, newFieldId) + scope.$digest() - expect(vm.submissionsBody.data.files[1].mediaType).to.equal('application/octet-stream'); + expect(vm.submissionsBody.data.files[1].mediaType).to.equal('application/octet-stream') var newFile2 = { name: 'source.zip', size: 2314, type: 'application/zip' - }; - var newFieldId2 = 'SOURCE_ZIP'; + } + var newFieldId2 = 'SOURCE_ZIP' - vm.setFileReference(newFile2, newFieldId2); - scope.$digest(); + vm.setFileReference(newFile2, newFieldId2) + scope.$digest() - expect(vm.submissionsBody.data.files[2].mediaType).to.equal('application/octet-stream'); - }); - }); + expect(vm.submissionsBody.data.files[2].mediaType).to.equal('application/octet-stream') + }) + }) describe('uploadSubmission', function() { it('adds comments to the submissions body', function() { - vm.comments = 'test comments'; - scope.$digest(); + vm.comments = 'test comments' + scope.$digest() - vm.uploadSubmission(); - scope.$digest(); + vm.uploadSubmission() + scope.$digest() - expect(vm.submissionsBody.data.submitterComments).to.equal('test comments'); - }); + expect(vm.submissionsBody.data.submitterComments).to.equal('test comments') + }) it('adds the rank to the submissions body', function() { - vm.submissionForm.submitterRank = 3; - scope.$digest(); + vm.submissionForm.submitterRank = 3 + scope.$digest() - vm.uploadSubmission(); - scope.$digest(); + vm.uploadSubmission() + scope.$digest() - expect(vm.submissionsBody.data.submitterRank).to.equal(3); - }); + expect(vm.submissionsBody.data.submitterRank).to.equal(3) + }) it('calls the submission service', function() { - var mockAPICall = sinon.spy(submissionsService, 'getPresignedURL'); + var mockAPICall = sinon.spy(submissionsService, 'getPresignedURL') - vm.uploadSubmission(); - scope.$digest(); + vm.uploadSubmission() + scope.$digest() - expect(mockAPICall).calledOnce; - }); + expect(mockAPICall).calledOnce + }) describe('processes the stockart and', function() { it('returns an empty array if no stockart given', function() { - vm.formStockarts = []; - scope.$digest(); + vm.formStockarts = [] + scope.$digest() - vm.uploadSubmission(); - scope.$digest(); + vm.uploadSubmission() + scope.$digest() - expect(vm.submissionsBody.data.stockArts).to.deep.equal([]); - }); + expect(vm.submissionsBody.data.stockArts).to.deep.equal([]) + }) it('removes the required properties and id from each stockart', function() { vm.formStockarts = [ @@ -216,7 +216,7 @@ describe('Submit Design Files Controller', function() { isPhotoURLRequired: false, isFileNumberRequired: false } - ]; + ] var processedStockart = [ { description: 'first stockart', @@ -228,25 +228,25 @@ describe('Submit Design Files Controller', function() { sourceUrl: 'url2.com', fileNumber: '234', } - ]; - scope.$digest(); + ] + scope.$digest() - vm.uploadSubmission(); - scope.$digest(); - expect(vm.submissionsBody.data.stockArts).to.deep.equal(processedStockart); + vm.uploadSubmission() + scope.$digest() + expect(vm.submissionsBody.data.stockArts).to.deep.equal(processedStockart) - }); - }); + }) + }) describe('processes the fonts and', function() { it('returns an empty array if no fonts given', function() { - vm.formFonts = []; - scope.$digest(); + vm.formFonts = [] + scope.$digest() - vm.uploadSubmission(); - scope.$digest(); + vm.uploadSubmission() + scope.$digest() - expect(vm.submissionsBody.data.fonts).to.deep.equal([]); - }); + expect(vm.submissionsBody.data.fonts).to.deep.equal([]) + }) it('removes the required properties and id from each font', function() { vm.formFonts = [ @@ -271,7 +271,7 @@ describe('Submit Design Files Controller', function() { isFontNameDisabled: true, isFontSourceRequired: false } - ]; + ] var processedFonts = [ { source: 'STUDIO_STANDARD_FONTS_LIST', @@ -282,37 +282,37 @@ describe('Submit Design Files Controller', function() { name: 'my other font', sourceUrl: 'fontsource.com', } - ]; - scope.$digest(); + ] + scope.$digest() - vm.uploadSubmission(); - scope.$digest(); - expect(vm.submissionsBody.data.fonts).to.deep.equal(processedFonts); - }); - }); - }); + vm.uploadSubmission() + scope.$digest() + expect(vm.submissionsBody.data.fonts).to.deep.equal(processedFonts) + }) + }) + }) describe('refreshPage', function() { it('reloads the page', function() { - var mockRefresh = sinon.spy(mockWindow.location, 'reload'); + var mockRefresh = sinon.spy(mockWindow.location, 'reload') - vm.refreshPage(); - scope.$digest(); + vm.refreshPage() + scope.$digest() - expect(mockRefresh).calledWith(true); - expect(mockRefresh).calledOnce; - }); - }); + expect(mockRefresh).calledWith(true) + expect(mockRefresh).calledOnce + }) + }) describe('cancelRetry', function() { it('sets showProgress to false', function() { - vm.showProgress = true; - scope.$digest(); - expect(vm.showProgress).to.be.true; - - vm.cancelRetry(); - scope.$digest(); - expect(vm.showProgress).to.be.false; - }); - }); -}); + vm.showProgress = true + scope.$digest() + expect(vm.showProgress).to.be.true + + vm.cancelRetry() + scope.$digest() + expect(vm.showProgress).to.be.false + }) + }) +})