From 7e4504edf2648a0c4cdb8636f727ebfd3a80bf6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonne=20Ha=C3=9F?= Date: Thu, 4 Dec 2014 18:13:56 +0100 Subject: [PATCH 1/3] Give the browser time to actually load the page before expecting to be on it --- features/support/matchers.rb | 17 +++++++++++++++++ features/support/paths.rb | 3 +-- 2 files changed, 18 insertions(+), 2 deletions(-) create mode 100644 features/support/matchers.rb diff --git a/features/support/matchers.rb b/features/support/matchers.rb new file mode 100644 index 000000000..17ce581e4 --- /dev/null +++ b/features/support/matchers.rb @@ -0,0 +1,17 @@ +RSpec::Matchers.define :have_path do |expected| + match do |actual| + start_time = Time.now + until actual.current_path == expected + return false if (Time.now-start_time) > Capybara.default_wait_time + sleep 0.05 + end + true + end + + failure_message_for_should do |actual| + "expected #{actual.inspect} to have path #{expected.inspect} but was #{actual.current_path.inspect}" + end + failure_message_for_should_not do |actual| + "expected #{actual.inspect} to not have path #{expected.inspect} but it had" + end +end diff --git a/features/support/paths.rb b/features/support/paths.rb index 8ccf11987..d1b923091 100644 --- a/features/support/paths.rb +++ b/features/support/paths.rb @@ -66,8 +66,7 @@ module NavigationHelpers end def confirm_on_page(page_name) - current_path = URI.parse(current_url).path - expect(current_path).to eq(path_to(page_name)) + expect(page).to have_path(path_to(page_name)) end end From 1c7e5b70266d9dfb06da077219e575e2730b202c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonne=20Ha=C3=9F?= Date: Thu, 4 Dec 2014 22:15:31 +0100 Subject: [PATCH 2/3] Improve apend_to_publisher - Use custom matcher that polls #value of the element - Wait for the text field to become visible before entering text --- features/support/matchers.rb | 31 +++++++++++++++++---- features/support/publishing_cuke_helpers.rb | 4 +-- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/features/support/matchers.rb b/features/support/matchers.rb index 17ce581e4..71a4bb600 100644 --- a/features/support/matchers.rb +++ b/features/support/matchers.rb @@ -1,11 +1,6 @@ RSpec::Matchers.define :have_path do |expected| match do |actual| - start_time = Time.now - until actual.current_path == expected - return false if (Time.now-start_time) > Capybara.default_wait_time - sleep 0.05 - end - true + await_condition { actual.current_path == expected } end failure_message_for_should do |actual| @@ -15,3 +10,27 @@ RSpec::Matchers.define :have_path do |expected| "expected #{actual.inspect} to not have path #{expected.inspect} but it had" end end + + +RSpec::Matchers.define :have_value do |expected| + match do |actual| + await_condition { actual.value && actual.value.include?(expected) } + end + + failure_message_for_should do |actual| + "expected #{actual.inspect} to have value #{expected.inspect} but was #{actual.value.inspect}" + end + failure_message_for_should_not do |actual| + "expected #{actual.inspect} to not have value #{expected.inspect} but it had" + end +end + + +def await_condition &condition + start_time = Time.now + until condition.call + return false if (Time.now-start_time) > Capybara.default_wait_time + sleep 0.05 + end + true +end diff --git a/features/support/publishing_cuke_helpers.rb b/features/support/publishing_cuke_helpers.rb index 7c15d6ff4..a49f3101c 100644 --- a/features/support/publishing_cuke_helpers.rb +++ b/features/support/publishing_cuke_helpers.rb @@ -4,11 +4,11 @@ module PublishingCukeHelpers end def append_to_publisher(txt, input_selector='#status_message_fake_text') - elem = find(input_selector, visible: false) + elem = find(input_selector) elem.native.send_keys(' ' + txt) # make sure the other text field got the new contents - expect(page).to have_xpath("//input|textarea/.[@id='status_message_text' and contains(@value, '#{txt}')]", visible: false) + expect(find("#status_message_text", visible: false)).to have_value txt end def upload_file_with_publisher(path) From 63e9d7b73f52b46c3582bdc0198de4a7d84d4536 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonne=20Ha=C3=9F?= Date: Fri, 5 Dec 2014 02:34:11 +0100 Subject: [PATCH 3/3] Update aspect list checkmarks only after the stream was fetched --- .../javascripts/app/models/stream_aspects.js | 9 ++++++++- .../javascripts/app/views/aspect_view.js | 2 +- .../app/views/aspects_list_view.js | 19 ++++++++++++------- features/step_definitions/aspects_steps.rb | 4 ++-- .../javascripts/app/views/aspect_view_spec.js | 1 - .../app/views/aspects_list_view_spec.js | 1 - 6 files changed, 23 insertions(+), 13 deletions(-) diff --git a/app/assets/javascripts/app/models/stream_aspects.js b/app/assets/javascripts/app/models/stream_aspects.js index 4f03fd3d1..a0115887d 100644 --- a/app/assets/javascripts/app/models/stream_aspects.js +++ b/app/assets/javascripts/app/models/stream_aspects.js @@ -21,7 +21,14 @@ app.models.StreamAspects = app.models.Stream.extend({ var url = this.url(); var ids = this.aspects_ids; this.deferred = this.items.fetch(this._fetchOpts({url : url, data : { 'a_ids': ids }})) - .done(_.bind(this.triggerFetchedEvents, this)); + .done(_.bind(this.fetchDone, this)); + }, + + fetchDone: function() { + this.triggerFetchedEvents(); + if (app.aspects) { + app.aspects.trigger('aspectStreamFetched'); + } } }); // @license-end diff --git a/app/assets/javascripts/app/views/aspect_view.js b/app/assets/javascripts/app/views/aspect_view.js index 3cef67458..982301370 100644 --- a/app/assets/javascripts/app/views/aspect_view.js +++ b/app/assets/javascripts/app/views/aspect_view.js @@ -14,7 +14,7 @@ app.views.Aspect = app.views.Base.extend({ toggleAspect: function(evt) { if (evt) { evt.preventDefault(); }; this.model.toggleSelected(); - this.$el.find('.icons-check_yes_ok').toggleClass('selected'); + app.router.aspects_stream(); }, diff --git a/app/assets/javascripts/app/views/aspects_list_view.js b/app/assets/javascripts/app/views/aspects_list_view.js index ea8d321bf..60416567c 100644 --- a/app/assets/javascripts/app/views/aspects_list_view.js +++ b/app/assets/javascripts/app/views/aspects_list_view.js @@ -12,6 +12,7 @@ app.views.AspectsList = app.views.Base.extend({ initialize: function() { this.collection.on('change', this.toggleSelector, this); this.collection.on('change', this.updateStreamTitle, this); + this.collection.on('aspectStreamFetched', this.updateAspectList, this); }, postRenderTemplate: function() { @@ -30,17 +31,10 @@ app.views.AspectsList = app.views.Base.extend({ toggleAll: function(evt) { if (evt) { evt.preventDefault(); }; - var aspects = this.$('li:not(:last)') if (this.collection.allSelected()) { this.collection.deselectAll(); - aspects.each(function(i){ - $(this).find('.icons-check_yes_ok').removeClass('selected'); - }); } else { this.collection.selectAll(); - aspects.each(function(i){ - $(this).find('.icons-check_yes_ok').addClass('selected'); - }); } this.toggleSelector(); @@ -60,6 +54,17 @@ app.views.AspectsList = app.views.Base.extend({ $('.stream_title').text(this.collection.toSentence()); }, + updateAspectList: function() { + this.collection.each(function(aspect) { + var element = this.$("li[data-aspect_id="+aspect.get('id')+"]"); + if (aspect.get('selected')) { + element.find('.icons-check_yes_ok').addClass('selected'); + } else { + element.find('.icons-check_yes_ok').removeClass('selected'); + } + }); + }, + hideAspectsList: function() { this.$el.empty(); }, diff --git a/features/step_definitions/aspects_steps.rb b/features/step_definitions/aspects_steps.rb index a151df7f0..759361f62 100644 --- a/features/step_definitions/aspects_steps.rb +++ b/features/step_definitions/aspects_steps.rb @@ -91,14 +91,14 @@ end Then /^I should see "([^"]*)" aspect selected$/ do |aspect_name| aspect = @me.aspects.where(:name => aspect_name).first within("#aspects_list") do - page.should have_css "li[data-aspect_id='#{aspect.id}'] .selected" + current_scope.should have_css "li[data-aspect_id='#{aspect.id}'] .selected" end end Then /^I should see "([^"]*)" aspect unselected$/ do |aspect_name| aspect = @me.aspects.where(:name => aspect_name).first within("#aspects_list") do - page.should have_no_css "li[data-aspect_id='#{aspect.id}'] .selected" + current_scope.should have_no_css "li[data-aspect_id='#{aspect.id}'] .selected" end end diff --git a/spec/javascripts/app/views/aspect_view_spec.js b/spec/javascripts/app/views/aspect_view_spec.js index 1247d6ad6..a69792092 100644 --- a/spec/javascripts/app/views/aspect_view_spec.js +++ b/spec/javascripts/app/views/aspect_view_spec.js @@ -28,7 +28,6 @@ describe("app.views.Aspect", function(){ it('it should deselect the aspect', function(){ this.view.$el.children('a.selectable').trigger('click'); expect(this.view.toggleAspect).toHaveBeenCalled(); - expect(this.view.$el.children('.icons-check_yes_ok').hasClass('selected')).toBeFalsy(); expect(app.router.aspects_stream).toHaveBeenCalled(); }); diff --git a/spec/javascripts/app/views/aspects_list_view_spec.js b/spec/javascripts/app/views/aspects_list_view_spec.js index e6bd915ec..6c18e3490 100644 --- a/spec/javascripts/app/views/aspects_list_view_spec.js +++ b/spec/javascripts/app/views/aspects_list_view_spec.js @@ -48,7 +48,6 @@ describe("app.views.AspectsList", function(){ it('should show all the aspects selected', function(){ expect(this.view.toggleAll).toHaveBeenCalled(); - expect(this.view.$('.selected').length).toBe(3); }); it('should show \'Deselect all\' link', function(){