From 4c13fd2b4264d8ca126c51bb3950d2ac18c79bcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20W=C3=B6rner?= Date: Wed, 14 Jan 2015 16:42:00 +0100 Subject: [PATCH 1/4] Revert "remove aspect sorting logic" This reverts commit 8d8d3c649a12f10270efcf9331c72110181999a7. Conflicts: app/assets/javascripts/main.js --- app/assets/javascripts/aspects-sorting.js | 18 ++++++++++++++++++ app/assets/javascripts/main.js | 2 ++ app/controllers/users_controller.rb | 2 ++ app/models/user.rb | 8 ++++++++ 4 files changed, 30 insertions(+) create mode 100644 app/assets/javascripts/aspects-sorting.js diff --git a/app/assets/javascripts/aspects-sorting.js b/app/assets/javascripts/aspects-sorting.js new file mode 100644 index 000000000..74d08e17a --- /dev/null +++ b/app/assets/javascripts/aspects-sorting.js @@ -0,0 +1,18 @@ +/* Copyright (c) 2010-2011, Diaspora Inc. This file is + * licensed under the Affero General Public License version 3 or later. See + * the COPYRIGHT file. + */ + +$(document).ready(function() { + $('#aspect_nav.left_nav .all_aspects .sub_nav').sortable({ + items: "li[data-aspect_id]", + update: function(event, ui) { + var order = $(this).sortable("toArray", {attribute: "data-aspect_id"}), + obj = { 'reorder_aspects': order, '_method': 'put' }; + $.ajax('/user', { type: 'post', dataType: 'script', data: obj }); + }, + revert: true, + helper: 'clone' + }); +}); + diff --git a/app/assets/javascripts/main.js b/app/assets/javascripts/main.js index aaa0c50ea..2dad8aea4 100644 --- a/app/assets/javascripts/main.js +++ b/app/assets/javascripts/main.js @@ -20,6 +20,7 @@ //= require jquery-idletimer/dist/idle-timer //= require jquery.infinitescroll-custom //= require jquery.autocomplete-custom +//= require jquery-ui //= require keycodes //= require fileuploader-custom //= require handlebars.runtime @@ -39,6 +40,7 @@ //= require_tree ./widgets //= require view //= require aspects-dropdown +//= require aspects-sorting //= require mentions //= require bootstrap-tooltip //= require bootstrap-popover diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 00932312a..dcffe700f 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -70,6 +70,8 @@ class UsersController < ApplicationController flash[:error] = I18n.t 'users.update.follow_settings_not_changed' end end + elsif aspect_order = params[:reorder_aspects] + @user.reorder_aspects(aspect_order) end set_email_preferences diff --git a/app/models/user.rb b/app/models/user.rb index b318465a2..f9c83c4ff 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -486,6 +486,14 @@ class User < ActiveRecord::Base end end + def reorder_aspects(aspect_order) + i = 0 + aspect_order.each do |id| + self.aspects.find(id).update_attributes({ :order_id => i }) + i += 1 + end + end + # Generate public/private keys for User and associated Person def generate_keys key_size = (Rails.env == 'test' ? 512 : 4096) From cee4f1c3cd51110214034bc45d60f27b8f577878 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20W=C3=B6rner?= Date: Wed, 14 Jan 2015 18:03:26 +0100 Subject: [PATCH 2/4] fixed aspect sorting, added minimal jquery ui, added test --- Gemfile | 1 + Gemfile.lock | 1 + app/assets/javascripts/aspects-sorting.js | 11 +++++------ app/assets/javascripts/main.js | 5 ++++- app/controllers/users_controller.rb | 6 +++--- spec/controllers/users_controller_spec.rb | 17 +++++++++++++++++ 6 files changed, 31 insertions(+), 10 deletions(-) diff --git a/Gemfile b/Gemfile index 42688dc40..ef221667f 100644 --- a/Gemfile +++ b/Gemfile @@ -86,6 +86,7 @@ gem "entypo-rails", "2.2.2" gem "backbone-on-rails", "1.1.2" gem "handlebars_assets", "0.20.1" gem "jquery-rails", "3.1.2" +gem "jquery-ui-rails", "5.0.3" gem "js_image_paths", "0.0.2" gem "js-routes", "1.0.0" diff --git a/Gemfile.lock b/Gemfile.lock index 87cfbd191..acdb4b639 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -739,6 +739,7 @@ DEPENDENCIES jasmine (= 2.2.0) jasmine-jquery-rails (= 2.0.3) jquery-rails (= 3.1.2) + jquery-ui-rails (= 5.0.3) js-routes (= 1.0.0) js_image_paths (= 0.0.2) jshintrb (= 0.3.0) diff --git a/app/assets/javascripts/aspects-sorting.js b/app/assets/javascripts/aspects-sorting.js index 74d08e17a..c6422f5a6 100644 --- a/app/assets/javascripts/aspects-sorting.js +++ b/app/assets/javascripts/aspects-sorting.js @@ -4,15 +4,14 @@ */ $(document).ready(function() { - $('#aspect_nav.left_nav .all_aspects .sub_nav').sortable({ - items: "li[data-aspect_id]", + $('#aspect_nav .nav').sortable({ + items: "li.aspect[data-aspect-id]", update: function(event, ui) { - var order = $(this).sortable("toArray", {attribute: "data-aspect_id"}), - obj = { 'reorder_aspects': order, '_method': 'put' }; - $.ajax('/user', { type: 'post', dataType: 'script', data: obj }); + var order = $(this).sortable("toArray", {attribute: "data-aspect-id"}), + obj = { 'aspect_order': order }; + $.ajax('/user', { type: 'put', dataType: 'text', data: obj }); }, revert: true, helper: 'clone' }); }); - diff --git a/app/assets/javascripts/main.js b/app/assets/javascripts/main.js index 2dad8aea4..1e5af284c 100644 --- a/app/assets/javascripts/main.js +++ b/app/assets/javascripts/main.js @@ -20,7 +20,10 @@ //= require jquery-idletimer/dist/idle-timer //= require jquery.infinitescroll-custom //= require jquery.autocomplete-custom -//= require jquery-ui +//= require jquery-ui/core +//= require jquery-ui/widget +//= require jquery-ui/mouse +//= require jquery-ui/sortable //= require keycodes //= require fileuploader-custom //= require handlebars.runtime diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index dcffe700f..aed49c31a 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -20,7 +20,9 @@ class UsersController < ApplicationController password_changed = false @user = current_user - if u = user_params + if aspect_order = params[:aspect_order] + @user.reorder_aspects(aspect_order) + elsif u = user_params # change email notifications if u[:email_preferences] @@ -70,8 +72,6 @@ class UsersController < ApplicationController flash[:error] = I18n.t 'users.update.follow_settings_not_changed' end end - elsif aspect_order = params[:reorder_aspects] - @user.reorder_aspects(aspect_order) end set_email_preferences diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 40006a3af..9d1d6c2bc 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -121,6 +121,23 @@ describe UsersController, :type => :controller do expect(response.status).to eq(204) end + describe 'aspect_order' do + it 'updates the aspect order' do + aspect_order = [] + @user.aspects.each do |aspect| + aspect.update_attributes({ :order_id => nil }) + aspect_order.push(aspect.id) + end + + put(:update, :id => @user.id, :aspect_order => aspect_order) + + @user.reload + @user.aspects.each do |aspect| + expect(aspect.order_id).not_to eq(nil) + end + end + end + describe 'password updates' do let(:password_params) do {:current_password => 'bluepin7', From 3c0975641762ec42132619892f4455b6492387db Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Sun, 26 Apr 2015 02:25:49 +0200 Subject: [PATCH 3/4] refactoring javascript and controller - move to contacts.js - use json - move to aspects_controller - add route - rewrite test - fix css in chrome --- app/assets/javascripts/app/pages/contacts.js | 14 ++++++++++++++ app/assets/javascripts/aspects-sorting.js | 17 ----------------- app/assets/javascripts/main.js | 1 - app/assets/stylesheets/contacts.scss | 1 + app/controllers/aspects_controller.rb | 7 +++++++ app/controllers/users_controller.rb | 4 +--- app/models/user.rb | 8 -------- config/routes.rb | 1 + spec/controllers/aspects_controller_spec.rb | 13 +++++++++++++ spec/controllers/users_controller_spec.rb | 17 ----------------- 10 files changed, 37 insertions(+), 46 deletions(-) delete mode 100644 app/assets/javascripts/aspects-sorting.js diff --git a/app/assets/javascripts/app/pages/contacts.js b/app/assets/javascripts/app/pages/contacts.js index 4ae44878b..9753825b6 100644 --- a/app/assets/javascripts/app/pages/contacts.js +++ b/app/assets/javascripts/app/pages/contacts.js @@ -22,6 +22,8 @@ app.pages.Contacts = Backbone.View.extend({ this.aspectCreateView = new app.views.AspectCreate({ el: $("#newAspectContainer") }); this.aspectCreateView.render(); + + this.setupAspectSorting(); }, toggleChatPrivilege: function() { @@ -73,6 +75,18 @@ app.pages.Contacts = Backbone.View.extend({ searchContactList: function(e) { this.stream.search($(e.target).val()); + }, + + setupAspectSorting: function() { + $("#aspect_nav .nav").sortable({ + items: "li.aspect[data-aspect-id]", + update: function() { + var data = JSON.stringify({ ordered_aspect_ids: $(this).sortable("toArray", { attribute: "data-aspect-id" }) }); + $.ajax("/aspects/order", { type: "put", dataType: "text", contentType: "application/json", data: data }); + }, + revert: true, + helper: "clone" + }); } }); // @license-end diff --git a/app/assets/javascripts/aspects-sorting.js b/app/assets/javascripts/aspects-sorting.js deleted file mode 100644 index c6422f5a6..000000000 --- a/app/assets/javascripts/aspects-sorting.js +++ /dev/null @@ -1,17 +0,0 @@ -/* Copyright (c) 2010-2011, Diaspora Inc. This file is - * licensed under the Affero General Public License version 3 or later. See - * the COPYRIGHT file. - */ - -$(document).ready(function() { - $('#aspect_nav .nav').sortable({ - items: "li.aspect[data-aspect-id]", - update: function(event, ui) { - var order = $(this).sortable("toArray", {attribute: "data-aspect-id"}), - obj = { 'aspect_order': order }; - $.ajax('/user', { type: 'put', dataType: 'text', data: obj }); - }, - revert: true, - helper: 'clone' - }); -}); diff --git a/app/assets/javascripts/main.js b/app/assets/javascripts/main.js index 1e5af284c..cc3a4f440 100644 --- a/app/assets/javascripts/main.js +++ b/app/assets/javascripts/main.js @@ -43,7 +43,6 @@ //= require_tree ./widgets //= require view //= require aspects-dropdown -//= require aspects-sorting //= require mentions //= require bootstrap-tooltip //= require bootstrap-popover diff --git a/app/assets/stylesheets/contacts.scss b/app/assets/stylesheets/contacts.scss index d43bd6eb7..9a356689d 100644 --- a/app/assets/stylesheets/contacts.scss +++ b/app/assets/stylesheets/contacts.scss @@ -64,6 +64,7 @@ #aspect_nav { li.aspect > a { padding-left: 30px; } li.new_aspect > a { padding-left: 30px; } + li.aspect.ui-sortable-handle.ui-sortable-helper { background: #FFF; } } #no_contacts { diff --git a/app/controllers/aspects_controller.rb b/app/controllers/aspects_controller.rb index 77e8bfc4a..5c6f5de79 100644 --- a/app/controllers/aspects_controller.rb +++ b/app/controllers/aspects_controller.rb @@ -64,6 +64,13 @@ class AspectsController < ApplicationController render :json => { :id => @aspect.id, :name => @aspect.name } end + def update_order + params[:ordered_aspect_ids].each_with_index do |id, i| + current_user.aspects.find(id).update_attributes(order_id: i) + end + render nothing: true + end + def toggle_chat_privilege @aspect = current_user.aspects.where(:id => params[:aspect_id]).first diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index aed49c31a..00932312a 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -20,9 +20,7 @@ class UsersController < ApplicationController password_changed = false @user = current_user - if aspect_order = params[:aspect_order] - @user.reorder_aspects(aspect_order) - elsif u = user_params + if u = user_params # change email notifications if u[:email_preferences] diff --git a/app/models/user.rb b/app/models/user.rb index f9c83c4ff..b318465a2 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -486,14 +486,6 @@ class User < ActiveRecord::Base end end - def reorder_aspects(aspect_order) - i = 0 - aspect_order.each do |id| - self.aspects.find(id).update_attributes({ :order_id => i }) - i += 1 - end - end - # Generate public/private keys for User and associated Person def generate_keys key_size = (Rails.env == 'test' ? 512 : 4096) diff --git a/config/routes.rb b/config/routes.rb index c7f924bae..140ae49af 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -63,6 +63,7 @@ Diaspora::Application.routes.draw do get "commented" => "streams#commented", :as => "commented_stream" get "aspects" => "streams#aspects", :as => "aspects_stream" + put "aspects/order" => "aspects#update_order" resources :aspects do put :toggle_contact_visibility put :toggle_chat_privilege diff --git a/spec/controllers/aspects_controller_spec.rb b/spec/controllers/aspects_controller_spec.rb index 65d413fde..779e510de 100644 --- a/spec/controllers/aspects_controller_spec.rb +++ b/spec/controllers/aspects_controller_spec.rb @@ -96,6 +96,19 @@ describe AspectsController, :type => :controller do end end + describe "update_order" do + it "updates the aspect order" do + @alices_aspect_1.update_attributes(order_id: 10) + @alices_aspect_2.update_attributes(order_id: 20) + ordered_aspect_ids = [@alices_aspect_2.id, @alices_aspect_1.id] + + put(:update_order, ordered_aspect_ids: ordered_aspect_ids) + + expect(Aspect.find(@alices_aspect_1.id).order_id).to eq(1) + expect(Aspect.find(@alices_aspect_2.id).order_id).to eq(0) + end + end + describe "#destroy" do before do @alices_aspect_1 = alice.aspects.create(name: "Contacts") diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 9d1d6c2bc..40006a3af 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -121,23 +121,6 @@ describe UsersController, :type => :controller do expect(response.status).to eq(204) end - describe 'aspect_order' do - it 'updates the aspect order' do - aspect_order = [] - @user.aspects.each do |aspect| - aspect.update_attributes({ :order_id => nil }) - aspect_order.push(aspect.id) - end - - put(:update, :id => @user.id, :aspect_order => aspect_order) - - @user.reload - @user.aspects.each do |aspect| - expect(aspect.order_id).not_to eq(nil) - end - end - end - describe 'password updates' do let(:password_params) do {:current_password => 'bluepin7', From 28c9cfdfd4379ee13c112f84f250d9e24a883baf Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Mon, 27 Apr 2015 02:19:52 +0200 Subject: [PATCH 4/4] write cucumber test and add order_id on create --- Gemfile | 5 +++++ Gemfile.lock | 5 +++++ app/assets/javascripts/app/pages/contacts.js | 5 ++++- app/models/aspect.rb | 4 ++++ app/views/contacts/_aspect_listings.haml | 2 +- config/routes.rb | 4 +++- features/desktop/manages_aspects.feature | 10 +++++++++ features/step_definitions/aspects_steps.rb | 23 ++++++++++++++++++++ spec/models/aspect_spec.rb | 7 ++++++ 9 files changed, 62 insertions(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index ef221667f..5727347e9 100644 --- a/Gemfile +++ b/Gemfile @@ -253,6 +253,11 @@ group :test do gem "database_cleaner" , "1.4.1" gem "selenium-webdriver", "2.45.0" + source "https://rails-assets.org" do + gem "rails-assets-jquery-simulate", "1.0.1" + gem "rails-assets-jquery-simulate-ext", "1.3.0" + end + # General helpers gem "factory_girl_rails", "4.5.0" diff --git a/Gemfile.lock b/Gemfile.lock index acdb4b639..3a3e50dd6 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -500,6 +500,9 @@ GEM rails-assets-jquery-idletimer (1.0.1) rails-assets-jquery-placeholder (2.1.1) rails-assets-jquery (>= 1.6) + rails-assets-jquery-simulate (1.0.1) + rails-assets-jquery-simulate-ext (1.3.0) + rails-assets-jquery (>= 1.7.0) rails-assets-jquery-textchange (0.2.3) rails-assets-jquery rails-assets-jquery-ui (1.10.4) @@ -773,6 +776,8 @@ DEPENDENCIES rails-assets-jquery (= 1.11.1)! rails-assets-jquery-idletimer (= 1.0.1)! rails-assets-jquery-placeholder (= 2.1.1)! + rails-assets-jquery-simulate (= 1.0.1)! + rails-assets-jquery-simulate-ext (= 1.3.0)! rails-assets-jquery-textchange (= 0.2.3)! rails-assets-markdown-it (= 4.2.0)! rails-assets-markdown-it--markdown-it-for-inline (= 0.1.0)! diff --git a/app/assets/javascripts/app/pages/contacts.js b/app/assets/javascripts/app/pages/contacts.js index 9753825b6..27e7ec126 100644 --- a/app/assets/javascripts/app/pages/contacts.js +++ b/app/assets/javascripts/app/pages/contacts.js @@ -81,8 +81,11 @@ app.pages.Contacts = Backbone.View.extend({ $("#aspect_nav .nav").sortable({ items: "li.aspect[data-aspect-id]", update: function() { + $("#aspect_nav .ui-sortable").removeClass("synced"); var data = JSON.stringify({ ordered_aspect_ids: $(this).sortable("toArray", { attribute: "data-aspect-id" }) }); - $.ajax("/aspects/order", { type: "put", dataType: "text", contentType: "application/json", data: data }); + $.ajax(Routes.order_aspects_path(), + { type: "put", dataType: "text", contentType: "application/json", data: data }) + .done(function() { $("#aspect_nav .ui-sortable").addClass("synced"); }); }, revert: true, helper: "clone" diff --git a/app/models/aspect.rb b/app/models/aspect.rb index 97c812f81..dfacdc811 100644 --- a/app/models/aspect.rb +++ b/app/models/aspect.rb @@ -20,6 +20,10 @@ class Aspect < ActiveRecord::Base name.strip! end + before_create do + self.order_id ||= Aspect.where(user_id: user_id).maximum(:order_id || 0).to_i + 1 + end + def to_s name end diff --git a/app/views/contacts/_aspect_listings.haml b/app/views/contacts/_aspect_listings.haml index 2e0d98945..664b412c6 100644 --- a/app/views/contacts/_aspect_listings.haml +++ b/app/views/contacts/_aspect_listings.haml @@ -1,5 +1,5 @@ #aspect_nav - %ul.nav.nav-tabs.nav-stacked + %ul.nav.nav-tabs.nav-stacked.synced %li.all_contacts{:class => ("active" if params["set"] == "all")} %a{:href => contacts_path(:set => "all")} = t('contacts.index.all_contacts') diff --git a/config/routes.rb b/config/routes.rb index 140ae49af..997c8f77f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -63,10 +63,12 @@ Diaspora::Application.routes.draw do get "commented" => "streams#commented", :as => "commented_stream" get "aspects" => "streams#aspects", :as => "aspects_stream" - put "aspects/order" => "aspects#update_order" resources :aspects do put :toggle_contact_visibility put :toggle_chat_privilege + collection do + put "order" => :update_order + end end get 'bookmarklet' => 'status_messages#bookmarklet' diff --git a/features/desktop/manages_aspects.feature b/features/desktop/manages_aspects.feature index 1d4f7e1df..5f6865eaf 100644 --- a/features/desktop/manages_aspects.feature +++ b/features/desktop/manages_aspects.feature @@ -75,3 +75,13 @@ Feature: User manages contacts And I click on my name in the header When I follow "Contacts" Then I should not see "Community spotlight" within ".span9" + + Scenario: sorting the aspects + Given I am signed in + And I have an aspect called "People" + And I have an aspect called "Cat People" + When I am on the contacts page + And I drag "Cat People" up 40 pixels + And I go to the contacts page + Then I should see "Cat People" as 2. aspect + And I should see "People" as 3. aspect diff --git a/features/step_definitions/aspects_steps.rb b/features/step_definitions/aspects_steps.rb index 620c56b40..5d7115066 100644 --- a/features/step_definitions/aspects_steps.rb +++ b/features/step_definitions/aspects_steps.rb @@ -88,6 +88,25 @@ When /^(.*) in the aspect creation modal$/ do |action| end end +When /^I drag "([^"]*)" (up|down) (\d+) pixels?$/ do |aspect_name, direction, distance| + distance = distance.to_i * -1 if direction == "up" + page.execute_script %{ + function drag() { + $("li.aspect:contains('#{aspect_name}')") + .simulate("drag-n-drop", { dy: #{distance}, interpolation: { stepWidth: 10, stepDelay: 5 } }); + } + function loadScripts() { + $.getScript("/assets/jquery-simulate/jquery.simulate.js", function(){ + $.getScript("/assets/jquery-simulate-ext/src/jquery.simulate.ext.js", function(){ + $.getScript("/assets/jquery-simulate-ext/src/jquery.simulate.drag-n-drop.js", drag); + }); + }); + } + if (!$.simulate) { loadScripts(); } else { drag(); } + } + expect(find("#aspect_nav")).to have_css ".synced" +end + And /^I toggle the aspect "([^"]*)"$/ do |name| toggle_aspect(name) end @@ -109,3 +128,7 @@ end Then /^the aspect dropdown should be visible$/ do aspect_dropdown_visible? end + +Then /^I should see "([^"]*)" as (\d+). aspect$/ do |aspect_name, position| + expect(find("#aspect_nav li:nth-child(#{position.to_i + 2})")).to have_text aspect_name +end diff --git a/spec/models/aspect_spec.rb b/spec/models/aspect_spec.rb index 4315345ae..4d85a2b1f 100644 --- a/spec/models/aspect_spec.rb +++ b/spec/models/aspect_spec.rb @@ -39,6 +39,13 @@ describe Aspect, :type => :model do it 'has a contacts_visible? method' do expect(alice.aspects.first.contacts_visible?).to be true end + + it "sets an order_id" do + aspect_2 = alice.aspects.create(name: "People") + expect(aspect_2.order_id).to eq(2) + aspect_3 = alice.aspects.create(name: "Cat People") + expect(aspect_3.order_id).to eq(3) + end end describe 'validation' do