diff --git a/app/assets/stylesheets/all.css.sass b/app/assets/stylesheets/all.css.sass index 1bdd089a1a5eb9a8ccaf55669d0602a307ff0503..f2ff416d5cdcba5dd27e933602ef851800f04e7b 100644 --- a/app/assets/stylesheets/all.css.sass +++ b/app/assets/stylesheets/all.css.sass @@ -63,8 +63,7 @@ h3.warning margin: 0 padding: 1em text-align: center - border-color: #de2b0f - background-color: #f04124 + background-color: orange +box-shadow(0 0 0.3em gray) +inline-block() +border-radius(1em) diff --git a/app/controllers/events_controller.rb b/app/controllers/events_controller.rb index 1da1f67106641f3e69b919b8251db9e631912c9c..0384ff45571f44e679602e476d84ace8ec458c1b 100644 --- a/app/controllers/events_controller.rb +++ b/app/controllers/events_controller.rb @@ -7,6 +7,7 @@ class EventsController < ApplicationController before_action :check_secret, only: [:edit, :preview, :update, :cancel, :destroy] before_action :set_mailer_host + rescue_from ActiveRecord::StaleObjectError, with: :locked def index respond_to do |format| @@ -105,8 +106,9 @@ class EventsController < ApplicationController # through. def event_params params.require(:event) - .permit :title, :start_time, :end_time, :description, :address, :city, - :region, :locality, :url, :contact, :submitter, :tags + .permit :lock_version, :title, :start_time, :end_time, :description, + :address, :city, :region, :locality, :url, :contact, :submitter, + :tags end # Check that you can only edit an existing event if you know its secret @@ -131,4 +133,9 @@ class EventsController < ApplicationController # Send an update mail to moderators ModerationMailer.update(@older_event, @event, nil).deliver end + + def locked + redirect_to edit_event_url(@event, secret: @event.secret), + alert: t('staleObjectError') + end end diff --git a/app/controllers/moderations_controller.rb b/app/controllers/moderations_controller.rb index 49f767ee232e888d2f5bde3f6e840ea04534edf7..4ad59fd7dddc9af76b7bcbd9eaf87283e437a13a 100644 --- a/app/controllers/moderations_controller.rb +++ b/app/controllers/moderations_controller.rb @@ -3,6 +3,7 @@ class ModerationsController < ApplicationController before_action :authenticate_user! before_action :set_moderation, :set_mailer_host, only: [:show, :edit, :preview, :update, :validate, :accept, :refuse, :destroy] + rescue_from ActiveRecord::StaleObjectError, with: :locked def index @events = Event.unmoderated @@ -19,7 +20,7 @@ class ModerationsController < ApplicationController def update @older_mod = Event.new @event.attributes respond_to do |format| - if @moderation.update(moderation_params) && send_moderation_mails + if @moderation.update_attributes(moderation_params) && send_mails format.html { redirect_to moderations_path, notice: t('.ok') } format.json { head :no_content } else @@ -71,8 +72,9 @@ class ModerationsController < ApplicationController # through. def moderation_params params.require(:event) - .permit :title, :start_time, :end_time, :description, :address, :city, - :region, :locality, :url, :contact, :submitter, :tags + .permit :lock_version, :title, :start_time, :end_time, :description, + :address, :city, :region, :locality, :url, :contact, :submitter, + :tags end # Useful to manage absolute url in mails @@ -80,7 +82,7 @@ class ModerationsController < ApplicationController ActionMailer::Base.default_url_options[:host] = request.host_with_port end - def send_moderation_mails + def send_mails # Send an update mail to moderators ModerationMailer.update(@older_mod, @moderation, current_user).deliver end @@ -112,4 +114,8 @@ class ModerationsController < ApplicationController @reason = t "moderations.refuse.reason_#{params[:reason]}_long" end end + + def locked + redirect_to edit_moderation_url(@moderation), alert: t('staleObjectError') + end end diff --git a/app/views/events/_form.html.haml b/app/views/events/_form.html.haml index 52198b070eeb5cbf6f421c1e7019b5634eacbe68..bd187c4f3407e573f2c6e52ade1d7cb1f6325f3e 100644 --- a/app/views/events/_form.html.haml +++ b/app/views/events/_form.html.haml @@ -1,13 +1,14 @@ = form_for @event, url: (@moderation ? moderation_path(@moderation) : @event.persisted? ? event_path(@event) : nil) do |f| + - if @event.persisted? + = f.hidden_field :lock_version + - unless @moderation + = hidden_field_tag :secret, params[:secret] + - if @event.errors.any? #flash_messages - @event.errors.full_messages.each do |msg| %p.flash.alert= msg - - if @event.persisted? - - unless @moderation - = hidden_field_tag :secret, params[:secret] - .field.title = f.label :title = f.text_field :title, required: true, size: 70, diff --git a/app/views/events/show.text.haml b/app/views/events/show.text.haml index f1bfe8e165f9d26c24f5cdc0687673befa0c0eb7..4b8a07ef7aacbbdccf95c2a11efe99bcbcfa3370 100644 --- a/app/views/events/show.text.haml +++ b/app/views/events/show.text.haml @@ -1,11 +1,11 @@ ===================================================== -#{Event.human_attribute_name(:locality).concat(':').ljust 12 } #{t "attributes.locality_#{@event.locality}"} #{Event.human_attribute_name(:title).concat(':').ljust 12 } #{@event.title} #{Event.human_attribute_name(:start_time).concat(':').ljust 12 } #{l @event.start_time, format: :at} #{Event.human_attribute_name(:end_time).concat(':').ljust 12 } #{l @event.end_time, format: :at} #{Event.human_attribute_name(:address).concat(':').ljust 12 } #{@event.address} #{Event.human_attribute_name(:city).concat(':').ljust 12 } #{@event.city} #{Event.human_attribute_name(:region).concat(':').ljust 12 } #{@event.related_region} +#{Event.human_attribute_name(:locality).concat(':').ljust 12 } #{t "attributes.locality_#{@event.locality}"} #{Event.human_attribute_name(:url).concat(':').ljust 12 } #{@event.url} #{Event.human_attribute_name(:contact).concat(':').ljust 12 } #{@event.contact} #{Event.human_attribute_name(:submitter).concat(':').ljust 12 } #{@event.submitter} diff --git a/app/views/moderations/edit.html.haml b/app/views/moderations/edit.html.haml index 531264e74bedadb6eb36d141212ef1e075745095..9edde274bb6dda0de952c090e72028093abe390d 100644 --- a/app/views/moderations/edit.html.haml +++ b/app/views/moderations/edit.html.haml @@ -3,7 +3,9 @@ =t '.title' - if @moderation.moderated? - %h3.warning=t '.warning' + %h3.warning + %em.fa.fa-warning + =t '.warning' %fieldset %legend diff --git a/config/locales/views/en.yml b/config/locales/views/en.yml index d9fbbb7bd28ba502565a87e1671d33a4a2e6d8ec..b75f7d9f306c76ce284816eeb8e75033e0465746 100644 --- a/config/locales/views/en.yml +++ b/config/locales/views/en.yml @@ -7,6 +7,8 @@ en: refuse: Refuse destroy: Destroy logout: Disconnect + staleObjectError: Sorry, your modification was rejected because someone else + already intervened # Translatings screens layouts: @@ -71,14 +73,14 @@ it more readable or agreable. It will appear online as soon as a moderator validates it. edit: title: Edit an event - warning: Warning, this event is already moderated. Any modification will - be immediately visible on the site. + warning: 'Event already moderated: any modification will be immediately + visible on the site' forbidden: You are not authorised to modify this event preview: Previsualisation edit: Edition preview: - warning: Warning, this event is already moderated. Any modification will - be immediately visible on the site. + warning: 'Event already moderated: any modification will be immediately + visible on the site' update: ok: Your event was updated form: @@ -104,8 +106,8 @@ it more readable or agreable. visualise: Visualise cancel: title: Cancel event - already_moderated: Warning, this event is already moderated. This - cancellation will remove it from Agenda du Libre. + already_moderated: 'Event already moderated: this cancellation will + remove it from Agenda du Libre' confirm: Do you confirm this event cancellation? preview: Event visualisation ok: Yes @@ -168,8 +170,8 @@ Example: `%{daylimit}`" edit: title: Edit event moderation: Moderation - warning: Warning, this event is already moderated. Any modification will - be immediately visible on the site. + warning: 'Event already moderated: any modification will be immediately + visible on the site' preview: Previsualisation edit: Edition update: diff --git a/config/locales/views/fr.yml b/config/locales/views/fr.yml index 3325197ca851440406cfca25e2c71001bb996e7f..490635b6a39f3383d56608104dd047e0173f75ec 100644 --- a/config/locales/views/fr.yml +++ b/config/locales/views/fr.yml @@ -7,6 +7,8 @@ fr: refuse: Refuser destroy: Supprimer logout: Se déconnecter + staleObjectError: Désolé, votre modification est rejetée car une autre + personne est déjà intervenue # Traductions des écrans layouts: @@ -63,28 +65,44 @@ fr: l'aura validé. edit: title: Éditer un événement - warning: Attention, cet événement est déjà modéré. Toute modification - sera immédiatement visible sur le site. + warning: 'Événement déjà modéré: toute modification sera immédiatement + visible sur le site' forbidden: Vous n'êtes pas authorisé à modifier cet événement preview: Prévisualisation edit: Édition preview: - warning: Attention, cet événement est déjà modéré. Toute modification - sera immédiatement visible sur le site. + warning: 'Événement déjà modéré: toute modification sera immédiatement + visible sur le site' update: ok: Votre événement a été mis à jour form: title_helper: Décrivez en moins de 5 mots votre événement, sans y indiquer le lieu, la ville ou la date + description_helper: Décrivez de la manière la plus complète possible + votre événement address_helper: "*Associée à la ville et la région, elle générerera une -carte [OpenStreetMap](http://www.openstreetmap.org), affichée aux côtés de -l'événement*" + carte [OpenStreetMap](http://www.openstreetmap.org), affichée aux côtés + de l'événement*" + url_helper: _Lien **direct** vers une page donnant plus d'informations + sur l'événement (lieu précis, horaire précis, programme précis...)_ + contact_helper: _Adresse e-mail de contact, affichée de manière peu + compréhensible par les spammeurs_ + submitter_helper: _Adresse e-mail du soumetteur de l'événement. Elle ne + sera utilisée que par les modérateurs pour contacter la personne ayant + proposé l'événement, pour l'informer de sa validation ou de son rejet. + Si cette adresse n'est pas présente, l'adresse de contact sera + utilisée_ + tags_helper: _Tags pour l'événement. Les tags sont séparés par des + espaces. Un tag ne peut contenir que des lettres minuscules, des + chiffres et des tirets. Dans les tags, indiquez le nom de la ou des + associations organisatrices. N'indiquez pas le nom de la ville ou de la + région._ save: Valider visualise: Visualiser cancel: title: Annulation de l'événement - already_moderated: Attention, cet événement est déjà modéré. Cette - annulation le fera disparaître de l'Agenda du Libre. + already_moderated: "Événement déjà modéré: cette annulation le fera + disparaître de l'Agenda du Libre" confirm: Confirmez-vous l'annulation de cet événement? preview: Visualisation de l'événement ok: Oui @@ -152,8 +170,8 @@ Exemple: `%{daylimit}`" edit: title: Éditer un événement moderation: Modération - warning: Attention, cet événement est déjà modéré. Toute modification - sera immédiatement visible sur le site. + warning: 'Événement déjà modéré: toute modification sera + immédiatement visible sur le site' preview: Prévisualisation edit: Édition update: diff --git a/db/migrate/20141011100700_add_lock_version_to_events.rb b/db/migrate/20141011100700_add_lock_version_to_events.rb new file mode 100644 index 0000000000000000000000000000000000000000..52870c23ea264d8a0ddfb858c4c3266bb8d310d7 --- /dev/null +++ b/db/migrate/20141011100700_add_lock_version_to_events.rb @@ -0,0 +1,7 @@ +# Add optimistic locking to events, so that moderators won't risk overwriting +# each others' work +class AddLockVersionToEvents < ActiveRecord::Migration + def change + add_column :events, :lock_version, :integer, default: 0, null: false + end +end diff --git a/db/schema.rb b/db/schema.rb index 99c7d3d5538e71575131cfeb46dfeb5aab6aa4c2..f472edd4f11d5a6489001a2b85c450e48c29eea7 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -12,7 +12,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20140823111115) do +ActiveRecord::Schema.define(version: 20141011100700) do create_table "active_admin_comments", force: true do |t| t.string "namespace" @@ -25,9 +25,9 @@ ActiveRecord::Schema.define(version: 20140823111115) do t.datetime "updated_at" end - add_index "active_admin_comments", ["author_type", "author_id"], name: "index_active_admin_comments_on_author_type_and_author_id" - add_index "active_admin_comments", ["namespace"], name: "index_active_admin_comments_on_namespace" - add_index "active_admin_comments", ["resource_type", "resource_id"], name: "index_active_admin_comments_on_resource_type_and_resource_id" + add_index "active_admin_comments", ["author_type", "author_id"], name: "index_active_admin_comments_on_author_type_and_author_id", using: :btree + add_index "active_admin_comments", ["namespace"], name: "index_active_admin_comments_on_namespace", using: :btree + add_index "active_admin_comments", ["resource_type", "resource_id"], name: "index_active_admin_comments_on_resource_type_and_resource_id", using: :btree create_table "admin_users", force: true do |t| t.string "email", default: "", null: false @@ -44,8 +44,8 @@ ActiveRecord::Schema.define(version: 20140823111115) do t.datetime "updated_at" end - add_index "admin_users", ["email"], name: "index_admin_users_on_email", unique: true - add_index "admin_users", ["reset_password_token"], name: "index_admin_users_on_reset_password_token", unique: true + add_index "admin_users", ["email"], name: "index_admin_users_on_email", unique: true, using: :btree + add_index "admin_users", ["reset_password_token"], name: "index_admin_users_on_reset_password_token", unique: true, using: :btree create_table "cities", force: true do |t| t.string "name", default: "", null: false @@ -57,7 +57,7 @@ ActiveRecord::Schema.define(version: 20140823111115) do t.float "longitude", limit: 24 end - add_index "cities", ["name"], name: "cities_name" + add_index "cities", ["name"], name: "cities_name", using: :btree create_table "events", force: true do |t| t.string "title", default: "", null: false @@ -73,16 +73,17 @@ ActiveRecord::Schema.define(version: 20140823111115) do t.integer "moderated", default: 0, null: false t.string "tags", default: "", null: false t.string "secret", default: "", null: false - t.datetime "decision_time", null: false - t.datetime "submission_time", null: false + t.datetime "decision_time" + t.datetime "submission_time" t.string "moderator_mail_id", limit: 32 t.string "submitter_mail_id", limit: 32 t.text "address" - t.float "latitude" - t.float "longitude" + t.float "latitude", limit: 24 + t.float "longitude", limit: 24 + t.integer "lock_version", default: 0, null: false end - add_index "events", ["start_time", "end_time"], name: "events_date" + add_index "events", ["start_time", "end_time"], name: "events_date", using: :btree create_table "lugs", force: true do |t| t.integer "region", default: 0, null: false diff --git a/test/controllers/events_controller_test.rb b/test/controllers/events_controller_test.rb index b885e5566efde6a641b732960cdf700fb33678ab..dfb4dac286e14c6e79756b496245d6cc1c2c96af 100644 --- a/test/controllers/events_controller_test.rb +++ b/test/controllers/events_controller_test.rb @@ -11,7 +11,7 @@ class EventsControllerTest < ActionController::TestCase test 'should get index' do get :index assert_response :success - assert_not_nil assigns(:events) + assert_not_nil assigns :events end test 'should get new' do @@ -23,8 +23,7 @@ class EventsControllerTest < ActionController::TestCase assert_no_difference 'Event.count' do post :preview_create, event: { title: @event.title, - start_time: @event.start_time, - end_time: @event.end_time, + start_time: @event.start_time, end_time: @event.end_time, description: @event.description, city: @event.city, region: @event.related_region, @@ -42,8 +41,7 @@ class EventsControllerTest < ActionController::TestCase assert_difference 'Event.count' do post :create, event: { title: @event.title, - start_time: @event.start_time, - end_time: @event.end_time, + start_time: @event.start_time, end_time: @event.end_time, description: @event.description, city: @event.city, region: @event.related_region, @@ -59,11 +57,7 @@ class EventsControllerTest < ActionController::TestCase test 'should not create event' do assert_no_difference 'Event.count' do - post :create, event: { - title: @event.title, - city: @event.city, - region: @event.related_region - } + post :create, event: { title: @event.title } assert_not_empty assigns(:event).errors.messages end @@ -75,7 +69,7 @@ class EventsControllerTest < ActionController::TestCase end test 'should get edit' do - get :edit, id: @event, secret: 'MyString' + get :edit, id: @event, secret: @event.secret assert_response :success end @@ -86,7 +80,7 @@ class EventsControllerTest < ActionController::TestCase test 'should preview' do assert_no_difference 'Event.count' do - patch :preview, id: @event, secret: 'MyString', event: { + patch :preview, id: @event, secret: @event.secret, event: { title: @event.title } @@ -97,7 +91,7 @@ class EventsControllerTest < ActionController::TestCase end test 'should update event' do - patch :update, id: @event, secret: 'MyString', event: { + patch :update, id: @event, secret: @event.secret, event: { title: @event.title } @@ -106,21 +100,29 @@ class EventsControllerTest < ActionController::TestCase end test 'should not update event' do - patch :update, id: @event, secret: 'MyString', event: { + patch :update, id: @event, secret: @event.secret, event: { title: nil } assert_not_empty assigns(:event).errors.messages end + test 'can not update event concurrently' do + patch :update, id: @event, secret: @event.secret, event: { + lock_version: @event.lock_version - 1 + } + + assert_redirected_to edit_event_url(@event, secret: @event.secret) + end + test 'should get cancel page' do - get :cancel, id: @event, secret: 'MyString' + get :cancel, id: @event, secret: @event.secret assert_response :success end test 'should destroy event' do assert_difference('Event.count', -1) do - delete :destroy, id: @event, secret: 'MyString' + delete :destroy, id: @event, secret: @event.secret end assert_redirected_to events_path diff --git a/test/controllers/moderations_controller_test.rb b/test/controllers/moderations_controller_test.rb index ecc6d78418f79d26bd850ef48d40765266fad151..6b0adb8db4fba9804f0c775876ae3b108b433ea2 100644 --- a/test/controllers/moderations_controller_test.rb +++ b/test/controllers/moderations_controller_test.rb @@ -58,7 +58,7 @@ class ModerationsControllerTest < ActionController::TestCase end test 'should update event' do - patch :update, id: @moderation, secret: 'MyString', event: { + patch :update, id: @moderation, event: { title: @moderation.title, start_time: @moderation.start_time, end_time: @moderation.end_time, @@ -73,13 +73,21 @@ class ModerationsControllerTest < ActionController::TestCase end test 'should not update event' do - patch :update, id: @moderation, secret: 'MyString', event: { + patch :update, id: @moderation, event: { title: nil } assert_not_empty assigns(:moderation).errors end + test 'can not update event concurrently' do + patch :update, id: @moderation, event: { + lock_version: @moderation.lock_version - 1 + } + + assert_redirected_to edit_moderation_path @moderation + end + test 'should reject event' do assert_difference 'Event.count', -1 do delete :destroy, id: @moderation diff --git a/test/fixtures/events.yml b/test/fixtures/events.yml index 3b14157da23d3c91f0d5b249b9a2210fb80855ed..fb1c969b67bd846e39002af43a4381e90b3eb89c 100644 --- a/test/fixtures/events.yml +++ b/test/fixtures/events.yml @@ -18,6 +18,7 @@ one: submission_time: <%= 3.days.ago %> moderator_mail_id: MyString submitter_mail_id: MyString + lock_version: 0 two: title: MyString @@ -37,6 +38,7 @@ two: submission_time: 2013-12-28 16:04:56 moderator_mail_id: MyString submitter_mail_id: MyString + lock_version: 0 proposed: title: MyString @@ -56,3 +58,4 @@ proposed: submission_time: 2013-12-28 16:04:56 moderator_mail_id: MyString submitter_mail_id: MyString + lock_version: 0