Address PR comments:
- Internationalize upload errors. - Move upload tests into their own files. - Refactor upload dialog acceptance tests.
This commit is contained in:
@@ -5,9 +5,11 @@ from lettuce import world, step
|
||||
from nose.tools import assert_true
|
||||
|
||||
from auth.authz import get_user_by_email, get_course_groupname_for_role
|
||||
from django.conf import settings
|
||||
|
||||
from selenium.webdriver.common.keys import Keys
|
||||
import time
|
||||
import os
|
||||
from django.contrib.auth.models import Group
|
||||
|
||||
from logging import getLogger
|
||||
@@ -15,6 +17,8 @@ logger = getLogger(__name__)
|
||||
|
||||
from terrain.browser import reset_data
|
||||
|
||||
TEST_ROOT = settings.COMMON_TEST_DATA_ROOT
|
||||
|
||||
########### STEP HELPERS ##############
|
||||
|
||||
|
||||
@@ -257,3 +261,12 @@ def type_in_codemirror(index, text):
|
||||
g._element.send_keys(text)
|
||||
if world.is_firefox():
|
||||
world.trigger_event('div.CodeMirror', index=index, event='blur')
|
||||
|
||||
|
||||
def upload_file(filename):
|
||||
file_css = '.upload-dialog input[type=file]'
|
||||
upload = world.css_find(file_css).first
|
||||
path = os.path.join(TEST_ROOT, filename)
|
||||
upload._element.send_keys(os.path.abspath(path))
|
||||
button_css = '.upload-dialog .action-upload'
|
||||
world.css_click(button_css)
|
||||
|
||||
@@ -76,6 +76,7 @@ Feature: Course Settings
|
||||
Scenario: User can upload course image
|
||||
Given I have opened a new course in Studio
|
||||
When I select Schedule and Details
|
||||
And I click the "Upload Course Image" button
|
||||
And I upload a new course image
|
||||
Then I should see the new course image
|
||||
And the image URL should be present in the field
|
||||
|
||||
@@ -4,9 +4,8 @@
|
||||
from lettuce import world, step
|
||||
from terrain.steps import reload_the_page
|
||||
from selenium.webdriver.common.keys import Keys
|
||||
from common import type_in_codemirror
|
||||
from common import type_in_codemirror, upload_file
|
||||
from django.conf import settings
|
||||
import os
|
||||
|
||||
from nose.tools import assert_true, assert_false, assert_equal
|
||||
|
||||
@@ -150,16 +149,15 @@ def test_change_course_overview(_step):
|
||||
type_in_codemirror(0, "<h1>Overview</h1>")
|
||||
|
||||
|
||||
@step('I click the "Upload Course Image" button')
|
||||
def click_upload_button(_step):
|
||||
button_css = '.action-upload-image'
|
||||
world.css_click(button_css)
|
||||
|
||||
|
||||
@step('I upload a new course image$')
|
||||
def upload_new_course_image(_step):
|
||||
upload_css = '.action-upload-image'
|
||||
world.css_click(upload_css)
|
||||
file_css = '.upload-dialog input[type=file]'
|
||||
upload = world.css_find(file_css)
|
||||
path = os.path.join(TEST_ROOT, 'image.jpg')
|
||||
upload._element.send_keys(os.path.abspath(path))
|
||||
button_css = '.upload-dialog .action-upload'
|
||||
world.css_click(button_css)
|
||||
upload_file('image.jpg')
|
||||
|
||||
|
||||
@step('I should see the new course image$')
|
||||
|
||||
@@ -3,7 +3,7 @@
|
||||
|
||||
from lettuce import world, step
|
||||
from django.conf import settings
|
||||
import os
|
||||
from common import upload_file
|
||||
|
||||
TEST_ROOT = settings.COMMON_TEST_DATA_ROOT
|
||||
|
||||
@@ -24,14 +24,8 @@ def assert_create_new_textbook_msg(_step):
|
||||
|
||||
|
||||
@step(u'I upload the textbook "([^"]*)"$')
|
||||
def upload_file(_step, file_name):
|
||||
file_css = '.upload-dialog input[type=file]'
|
||||
upload = world.css_find(file_css)
|
||||
# uploading the file itself
|
||||
path = os.path.join(TEST_ROOT, 'uploads', file_name)
|
||||
upload._element.send_keys(os.path.abspath(path))
|
||||
button_css = ".upload-dialog .action-upload"
|
||||
world.css_click(button_css)
|
||||
def upload_textbook(_step, file_name):
|
||||
upload_file(file_name)
|
||||
|
||||
|
||||
@step(u'I click (on )?the New Textbook button')
|
||||
|
||||
@@ -196,32 +196,3 @@ describe "CMS.Collections.ChapterSet", ->
|
||||
# try going back one
|
||||
@collection.remove(@collection.last())
|
||||
expect(@collection.nextOrder()).toEqual(2)
|
||||
|
||||
|
||||
describe "CMS.Models.FileUpload", ->
|
||||
beforeEach ->
|
||||
@model = new CMS.Models.FileUpload()
|
||||
|
||||
it "is unfinished by default", ->
|
||||
expect(@model.get("finished")).toBeFalsy()
|
||||
|
||||
it "is not uploading by default", ->
|
||||
expect(@model.get("uploading")).toBeFalsy()
|
||||
|
||||
it "is valid by default", ->
|
||||
expect(@model.isValid()).toBeTruthy()
|
||||
|
||||
it "is valid for PDF files", ->
|
||||
file = {"type": "application/pdf"}
|
||||
@model.set("selectedFile", file);
|
||||
expect(@model.isValid()).toBeTruthy()
|
||||
|
||||
it "is invalid for text files", ->
|
||||
file = {"type": "text/plain"}
|
||||
@model.set("selectedFile", file);
|
||||
expect(@model.isValid()).toBeFalsy()
|
||||
|
||||
it "is invalid for PNG files", ->
|
||||
file = {"type": "image/png"}
|
||||
@model.set("selectedFile", file);
|
||||
expect(@model.isValid()).toBeFalsy()
|
||||
|
||||
33
cms/static/coffee/spec/models/upload_spec.coffee
Normal file
33
cms/static/coffee/spec/models/upload_spec.coffee
Normal file
@@ -0,0 +1,33 @@
|
||||
describe "CMS.Models.FileUpload", ->
|
||||
beforeEach ->
|
||||
@model = new CMS.Models.FileUpload()
|
||||
|
||||
it "is unfinished by default", ->
|
||||
expect(@model.get("finished")).toBeFalsy()
|
||||
|
||||
it "is not uploading by default", ->
|
||||
expect(@model.get("uploading")).toBeFalsy()
|
||||
|
||||
it "is valid by default", ->
|
||||
expect(@model.isValid()).toBeTruthy()
|
||||
|
||||
it "is valid for PDF files by default", ->
|
||||
file = {"type": "application/pdf"}
|
||||
@model.set("selectedFile", file);
|
||||
expect(@model.isValid()).toBeTruthy()
|
||||
|
||||
it "is invalid for text files by default", ->
|
||||
file = {"type": "text/plain"}
|
||||
@model.set("selectedFile", file);
|
||||
expect(@model.isValid()).toBeFalsy()
|
||||
|
||||
it "is invalid for PNG files by default", ->
|
||||
file = {"type": "image/png"}
|
||||
@model.set("selectedFile", file);
|
||||
expect(@model.isValid()).toBeFalsy()
|
||||
|
||||
it "can accept non-PDF files when explicitly set", ->
|
||||
file = {"type": "image/png"}
|
||||
@model.set("mimeType": "image/png")
|
||||
@model.set("selectedFile", file)
|
||||
expect(@model.isValid()).toBeTruthy()
|
||||
@@ -311,121 +311,3 @@ describe "CMS.Views.EditChapter", ->
|
||||
@view.$(".action-upload").click()
|
||||
expect(@model.get("name")).toEqual("rainbows")
|
||||
expect(@model.get("asset_path")).toEqual("unicorns")
|
||||
|
||||
|
||||
describe "CMS.Views.UploadDialog", ->
|
||||
tpl = readFixtures("upload-dialog.underscore")
|
||||
|
||||
beforeEach ->
|
||||
setFixtures($("<script>", {id: "upload-dialog-tpl", type: "text/template"}).text(tpl))
|
||||
appendSetFixtures($("<script>", {id: "system-feedback-tpl", type: "text/template"}).text(feedbackTpl))
|
||||
CMS.URL.UPLOAD_ASSET = "/upload"
|
||||
|
||||
@model = new CMS.Models.FileUpload()
|
||||
@chapter = new CMS.Models.Chapter()
|
||||
@view = new CMS.Views.UploadDialog(
|
||||
model: @model
|
||||
onSuccess: (response) =>
|
||||
options = {}
|
||||
if !@chapter.get('name')
|
||||
options.name = response.displayname
|
||||
options.asset_path = response.url
|
||||
@chapter.set(options)
|
||||
)
|
||||
spyOn(@view, 'remove').andCallThrough()
|
||||
|
||||
# create mock file input, so that we aren't subject to browser restrictions
|
||||
@mockFiles = []
|
||||
mockFileInput = jasmine.createSpy('mockFileInput')
|
||||
mockFileInput.files = @mockFiles
|
||||
jqMockFileInput = jasmine.createSpyObj('jqMockFileInput', ['get', 'replaceWith'])
|
||||
jqMockFileInput.get.andReturn(mockFileInput)
|
||||
realMethod = @view.$
|
||||
spyOn(@view, "$").andCallFake (selector) ->
|
||||
if selector == "input[type=file]"
|
||||
jqMockFileInput
|
||||
else
|
||||
realMethod.apply(this, arguments)
|
||||
|
||||
afterEach ->
|
||||
delete CMS.URL.UPLOAD_ASSET
|
||||
|
||||
describe "Basic", ->
|
||||
it "should be shown by default", ->
|
||||
expect(@view.options.shown).toBeTruthy()
|
||||
|
||||
it "should render without a file selected", ->
|
||||
@view.render()
|
||||
expect(@view.$el).toContain("input[type=file]")
|
||||
expect(@view.$(".action-upload")).toHaveClass("disabled")
|
||||
|
||||
it "should render with a PDF selected", ->
|
||||
file = {name: "fake.pdf", "type": "application/pdf"}
|
||||
@mockFiles.push(file)
|
||||
@model.set("selectedFile", file)
|
||||
@view.render()
|
||||
expect(@view.$el).toContain("input[type=file]")
|
||||
expect(@view.$el).not.toContain("#upload_error")
|
||||
expect(@view.$(".action-upload")).not.toHaveClass("disabled")
|
||||
|
||||
it "should render an error with an invalid file type selected", ->
|
||||
file = {name: "fake.png", "type": "image/png"}
|
||||
@mockFiles.push(file)
|
||||
@model.set("selectedFile", file)
|
||||
@view.render()
|
||||
expect(@view.$el).toContain("input[type=file]")
|
||||
expect(@view.$el).toContain("#upload_error")
|
||||
expect(@view.$(".action-upload")).toHaveClass("disabled")
|
||||
|
||||
|
||||
it "adds body class on show()", ->
|
||||
@view.show()
|
||||
expect(@view.options.shown).toBeTruthy()
|
||||
# can't test: this blows up the spec runner
|
||||
# expect($("body")).toHaveClass("dialog-is-shown")
|
||||
|
||||
it "removes body class on hide()", ->
|
||||
@view.hide()
|
||||
expect(@view.options.shown).toBeFalsy()
|
||||
# can't test: this blows up the spec runner
|
||||
# expect($("body")).not.toHaveClass("dialog-is-shown")
|
||||
|
||||
describe "Uploads", ->
|
||||
beforeEach ->
|
||||
@requests = requests = []
|
||||
@xhr = sinon.useFakeXMLHttpRequest()
|
||||
@xhr.onCreate = (xhr) -> requests.push(xhr)
|
||||
@clock = sinon.useFakeTimers()
|
||||
|
||||
afterEach ->
|
||||
@xhr.restore()
|
||||
@clock.restore()
|
||||
|
||||
it "can upload correctly", ->
|
||||
@view.upload()
|
||||
expect(@model.get("uploading")).toBeTruthy()
|
||||
expect(@requests.length).toEqual(1)
|
||||
request = @requests[0]
|
||||
expect(request.url).toEqual("/upload")
|
||||
expect(request.method).toEqual("POST")
|
||||
|
||||
request.respond(200, {"Content-Type": "application/json"},
|
||||
'{"displayname": "starfish", "url": "/uploaded/starfish.pdf"}')
|
||||
expect(@model.get("uploading")).toBeFalsy()
|
||||
expect(@model.get("finished")).toBeTruthy()
|
||||
expect(@chapter.get("name")).toEqual("starfish")
|
||||
expect(@chapter.get("asset_path")).toEqual("/uploaded/starfish.pdf")
|
||||
|
||||
it "can handle upload errors", ->
|
||||
@view.upload()
|
||||
@requests[0].respond(500)
|
||||
expect(@model.get("title")).toMatch(/error/)
|
||||
expect(@view.remove).not.toHaveBeenCalled()
|
||||
|
||||
it "removes itself after two seconds on successful upload", ->
|
||||
@view.upload()
|
||||
@requests[0].respond(200, {"Content-Type": "application/json"},
|
||||
'{"displayname": "starfish", "url": "/uploaded/starfish.pdf"}')
|
||||
expect(@view.remove).not.toHaveBeenCalled()
|
||||
@clock.tick(2001)
|
||||
expect(@view.remove).toHaveBeenCalled()
|
||||
|
||||
118
cms/static/coffee/spec/views/upload_spec.coffee
Normal file
118
cms/static/coffee/spec/views/upload_spec.coffee
Normal file
@@ -0,0 +1,118 @@
|
||||
feedbackTpl = readFixtures('system-feedback.underscore')
|
||||
|
||||
describe "CMS.Views.UploadDialog", ->
|
||||
tpl = readFixtures("upload-dialog.underscore")
|
||||
|
||||
beforeEach ->
|
||||
setFixtures($("<script>", {id: "upload-dialog-tpl", type: "text/template"}).text(tpl))
|
||||
appendSetFixtures($("<script>", {id: "system-feedback-tpl", type: "text/template"}).text(feedbackTpl))
|
||||
CMS.URL.UPLOAD_ASSET = "/upload"
|
||||
|
||||
@model = new CMS.Models.FileUpload()
|
||||
@chapter = new CMS.Models.Chapter()
|
||||
@view = new CMS.Views.UploadDialog(
|
||||
model: @model
|
||||
onSuccess: (response) =>
|
||||
options = {}
|
||||
if !@chapter.get('name')
|
||||
options.name = response.displayname
|
||||
options.asset_path = response.url
|
||||
@chapter.set(options)
|
||||
)
|
||||
spyOn(@view, 'remove').andCallThrough()
|
||||
|
||||
# create mock file input, so that we aren't subject to browser restrictions
|
||||
@mockFiles = []
|
||||
mockFileInput = jasmine.createSpy('mockFileInput')
|
||||
mockFileInput.files = @mockFiles
|
||||
jqMockFileInput = jasmine.createSpyObj('jqMockFileInput', ['get', 'replaceWith'])
|
||||
jqMockFileInput.get.andReturn(mockFileInput)
|
||||
realMethod = @view.$
|
||||
spyOn(@view, "$").andCallFake (selector) ->
|
||||
if selector == "input[type=file]"
|
||||
jqMockFileInput
|
||||
else
|
||||
realMethod.apply(this, arguments)
|
||||
|
||||
afterEach ->
|
||||
delete CMS.URL.UPLOAD_ASSET
|
||||
|
||||
describe "Basic", ->
|
||||
it "should be shown by default", ->
|
||||
expect(@view.options.shown).toBeTruthy()
|
||||
|
||||
it "should render without a file selected", ->
|
||||
@view.render()
|
||||
expect(@view.$el).toContain("input[type=file]")
|
||||
expect(@view.$(".action-upload")).toHaveClass("disabled")
|
||||
|
||||
it "should render with a PDF selected", ->
|
||||
file = {name: "fake.pdf", "type": "application/pdf"}
|
||||
@mockFiles.push(file)
|
||||
@model.set("selectedFile", file)
|
||||
@view.render()
|
||||
expect(@view.$el).toContain("input[type=file]")
|
||||
expect(@view.$el).not.toContain("#upload_error")
|
||||
expect(@view.$(".action-upload")).not.toHaveClass("disabled")
|
||||
|
||||
it "should render an error with an invalid file type selected", ->
|
||||
file = {name: "fake.png", "type": "image/png"}
|
||||
@mockFiles.push(file)
|
||||
@model.set("selectedFile", file)
|
||||
@view.render()
|
||||
expect(@view.$el).toContain("input[type=file]")
|
||||
expect(@view.$el).toContain("#upload_error")
|
||||
expect(@view.$(".action-upload")).toHaveClass("disabled")
|
||||
|
||||
|
||||
it "adds body class on show()", ->
|
||||
@view.show()
|
||||
expect(@view.options.shown).toBeTruthy()
|
||||
# can't test: this blows up the spec runner
|
||||
# expect($("body")).toHaveClass("dialog-is-shown")
|
||||
|
||||
it "removes body class on hide()", ->
|
||||
@view.hide()
|
||||
expect(@view.options.shown).toBeFalsy()
|
||||
# can't test: this blows up the spec runner
|
||||
# expect($("body")).not.toHaveClass("dialog-is-shown")
|
||||
|
||||
describe "Uploads", ->
|
||||
beforeEach ->
|
||||
@requests = requests = []
|
||||
@xhr = sinon.useFakeXMLHttpRequest()
|
||||
@xhr.onCreate = (xhr) -> requests.push(xhr)
|
||||
@clock = sinon.useFakeTimers()
|
||||
|
||||
afterEach ->
|
||||
@xhr.restore()
|
||||
@clock.restore()
|
||||
|
||||
it "can upload correctly", ->
|
||||
@view.upload()
|
||||
expect(@model.get("uploading")).toBeTruthy()
|
||||
expect(@requests.length).toEqual(1)
|
||||
request = @requests[0]
|
||||
expect(request.url).toEqual("/upload")
|
||||
expect(request.method).toEqual("POST")
|
||||
|
||||
request.respond(200, {"Content-Type": "application/json"},
|
||||
'{"displayname": "starfish", "url": "/uploaded/starfish.pdf"}')
|
||||
expect(@model.get("uploading")).toBeFalsy()
|
||||
expect(@model.get("finished")).toBeTruthy()
|
||||
expect(@chapter.get("name")).toEqual("starfish")
|
||||
expect(@chapter.get("asset_path")).toEqual("/uploaded/starfish.pdf")
|
||||
|
||||
it "can handle upload errors", ->
|
||||
@view.upload()
|
||||
@requests[0].respond(500)
|
||||
expect(@model.get("title")).toMatch(/error/)
|
||||
expect(@view.remove).not.toHaveBeenCalled()
|
||||
|
||||
it "removes itself after two seconds on successful upload", ->
|
||||
@view.upload()
|
||||
@requests[0].respond(200, {"Content-Type": "application/json"},
|
||||
'{"displayname": "starfish", "url": "/uploaded/starfish.pdf"}')
|
||||
expect(@view.remove).not.toHaveBeenCalled()
|
||||
@clock.tick(2001)
|
||||
expect(@view.remove).toHaveBeenCalled()
|
||||
@@ -15,7 +15,12 @@ CMS.Models.FileUpload = Backbone.Model.extend({
|
||||
validate: function(attrs, options) {
|
||||
if(attrs.selectedFile && attrs.selectedFile.type !== this.attributes.mimeType) {
|
||||
return {
|
||||
message: "Only " + this.attributes.fileType + " files can be uploaded. Please select a file ending in ." + this.attributes.fileType.toLowerCase() + " to upload.",
|
||||
message: _.template(
|
||||
gettext("Only {fileType} files can be uploaded. Please select a file ending in .{fileExtension} to upload."),
|
||||
{
|
||||
fileType: this.attributes.fileType,
|
||||
fileExtension: this.attributes.fileType.toLowerCase()
|
||||
}),
|
||||
attributes: {selectedFile: true}
|
||||
};
|
||||
}
|
||||
|
||||
@@ -11,7 +11,7 @@
|
||||
<h2 class="title"><%= title %></h2>
|
||||
<% if(error) {%>
|
||||
<div id="upload_error" class="message message-status message-status error is-shown" name="upload_error">
|
||||
<p><%= gettext(error.message) %></p>
|
||||
<p><%= error.message %></p>
|
||||
</div>
|
||||
<% } %>
|
||||
<p id="dialog-assetupload-description" class="message"><%= message %></p>
|
||||
|
||||
@@ -48,7 +48,7 @@ from contentstore import utils
|
||||
reset: true
|
||||
});
|
||||
|
||||
CMS.URL.UPLOAD_ASSET = '${upload_asset_url}';
|
||||
CMS.URL.UPLOAD_ASSET = '${upload_asset_url}';
|
||||
});
|
||||
|
||||
</script>
|
||||
|
||||
Reference in New Issue
Block a user