From 0c81809edfb4a2f06cb036948d5269d558e486b1 Mon Sep 17 00:00:00 2001 From: f Date: Wed, 11 Dec 2019 17:05:31 -0300 Subject: [PATCH] cambiar el algoritmo de ordenamiento MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit para poder respetar el orden de más nuevo a más antiguo, el número de orden se convirtió en decreciente (de más alto a más bajo). además, encontramos algunos temas de performance como guardar solo los artículos que cambiaron y no todos. se graban los cambios sin validarlos, es decir, solo el cambio de orden. --- app/controllers/posts_controller.rb | 2 +- app/javascript/packs/application.js | 8 +- app/models/metadata_order.rb | 5 +- app/models/metadata_template.rb | 2 + app/models/post.rb | 13 +-- app/models/post_relation.rb | 17 +++- app/models/site/static_file_migration.rb | 2 +- app/services/post_service.rb | 13 ++- app/views/posts/index.haml | 115 +++++++++++------------ doc/reordenar.md | 8 ++ test/models/post_test.rb | 2 +- 11 files changed, 105 insertions(+), 82 deletions(-) diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index c9ea8d23..fe8ad47d 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -12,7 +12,7 @@ class PostsController < ApplicationController @layout = params.dig(:layout).try :to_sym # TODO: Aplicar policy_scope @posts = @site.posts(lang: I18n.locale) - @posts.sort_by! :order, :date + @posts.sort_by!(:order, :date).reverse! end def show diff --git a/app/javascript/packs/application.js b/app/javascript/packs/application.js index 6d8e02e1..e8b19a03 100644 --- a/app/javascript/packs/application.js +++ b/app/javascript/packs/application.js @@ -29,7 +29,11 @@ document.addEventListener('turbolinks:load', () => { onlyBody: true, dragHandler: '.handle' }).on('drop', (from, to, el, mode) => { - $('.reorder').val((i,v) => i); - $('.submit-reorder').removeClass('d-none'); + Array.from(document.querySelectorAll('.reorder')) + .reverse() + .map((o,i) => o.value = i); + + Array.from(document.querySelectorAll('.submit-reorder')) + .map(s => s.classList.remove('d-none')); }); }) diff --git a/app/models/metadata_order.rb b/app/models/metadata_order.rb index df9e0559..e1f66e68 100644 --- a/app/models/metadata_order.rb +++ b/app/models/metadata_order.rb @@ -2,8 +2,9 @@ # Un campo de orden class MetadataOrder < MetadataTemplate - # El valor es 0 porque estamos ordenando por orden y fecha + # El valor según la posición del post en la relación ordenada por + # fecha, a fecha más alta, posición más alta def default_value - 0 + site.posts(lang: post.lang.value).sort_by(:date).index(post) end end diff --git a/app/models/metadata_template.rb b/app/models/metadata_template.rb index de5007b9..fb8e0388 100644 --- a/app/models/metadata_template.rb +++ b/app/models/metadata_template.rb @@ -15,6 +15,8 @@ MetadataTemplate = Struct.new(:site, :document, :name, :label, :type, # Valores posibles, busca todos los valores actuales en otros # artículos del mismo sitio + # + # TODO: Implementar lang! def values site.everything_of(name) end diff --git a/app/models/post.rb b/app/models/post.rb index 0dd9299f..8daf9ed1 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -156,8 +156,8 @@ class Post < OpenStruct # Guarda los cambios # rubocop:disable Metrics/CyclomaticComplexity - def save(validation = true) - return false if validation && !valid? + def save(validate: true) + return false if validate && !valid? # Salir si tenemos que cambiar el nombre del archivo y no pudimos return false if !new? && path_changed? && !update_path! return false unless save_attributes! @@ -197,12 +197,6 @@ class Post < OpenStruct # Detecta si el artículo es válido para guardar def valid? - validate - errors.blank? - end - - # Requisitos para que el post sea válido - def validate self.errors = {} layout.metadata.keys.map(&:to_sym).each do |metadata| @@ -210,8 +204,9 @@ class Post < OpenStruct errors[metadata] = template.errors unless template.valid? end + + errors.blank? end - alias validate! validate # Guarda los cambios en el archivo destino def write diff --git a/app/models/post_relation.rb b/app/models/post_relation.rb index 22eb5ca9..934b2c54 100644 --- a/app/models/post_relation.rb +++ b/app/models/post_relation.rb @@ -31,15 +31,18 @@ class PostRelation < Array post end + alias sort_by_generic sort_by alias sort_by_generic! sort_by! # Permite ordenar los artículos por sus atributos # # XXX: Prestar atención cuando estamos mezclando artículos con # diferentes tipos de atributos. - def sort_by!(*attrs) - sort_by_generic! do |post| + def sort_by(*attrs) + sort_by_generic do |post| attrs.map do |attr| + # TODO: detectar el tipo de atributo faltante y obtener el valor + # por defecto para hacer la comparación return 0 unless post.attributes.include? attr post.public_send(attr).value @@ -47,6 +50,10 @@ class PostRelation < Array end end + def sort_by!(*attrs) + replace sort_by(*attrs) + end + alias find_generic find # Encontra un post por su id convertido a SHA1 @@ -65,8 +72,10 @@ class PostRelation < Array end # Intenta guardar todos y devuelve true si pudo - def save_all - map(&:save).all? + def save_all(validate: true) + map do |post| + post.save(validate: validate) + end.all? end private diff --git a/app/models/site/static_file_migration.rb b/app/models/site/static_file_migration.rb index 4d08fc8c..48b3cf28 100644 --- a/app/models/site/static_file_migration.rb +++ b/app/models/site/static_file_migration.rb @@ -79,7 +79,7 @@ class Site end # Guardamos los cambios - unless doc.save(false) + unless doc.save(validate: false) log.write "#{doc.path.relative} no se pudo guardar\n" end diff --git a/app/services/post_service.rb b/app/services/post_service.rb index 0f6bac19..b97a3fdd 100644 --- a/app/services/post_service.rb +++ b/app/services/post_service.rb @@ -47,18 +47,23 @@ PostService = Struct.new(:site, :usuarie, :post, :params, keyword_init: true) do def reorder posts = site.posts(lang: lang) reorder = params.require(:post).permit(reorder: {}).try(:[], :reorder) + modified = PostRelation.new(site: site) files = reorder.keys.map do |id| post = posts.find(id, sha1: true) - next unless post.attributes.include? :order + order = reorder[id].to_i - post.usuaries << usuarie - post.order.value = reorder[id].to_i + next unless post.attributes.include? :order + next if post.order.value == order + + modified << post + post.order.value = order post.path.absolute end.compact # TODO: Implementar transacciones! - posts.save_all && commit(action: :reorder, file: files) + modified.save_all(validate: false) && + commit(action: :reorder, file: files) end private diff --git a/app/views/posts/index.haml b/app/views/posts/index.haml index 4c903485..e50bdfa8 100644 --- a/app/views/posts/index.haml +++ b/app/views/posts/index.haml @@ -25,62 +25,61 @@ %section.col = render 'layouts/flash' - - if @posts.present? - .row - .col - = form_tag site_posts_reorder_path, method: :post do - = submit_tag t('posts.reorder'), class: 'btn submit-reorder d-none' - -# TODO: Permitir cambiar el idioma - %table.table.table-condensed.table-draggable - %tbody - - @posts.each_with_index do |post, i| - -# - saltearse el post a menos que esté en la categoría por - la que estamos filtrando - - if @category - - next unless post.attributes.include? :categories - - next unless post.categories.value.include?(@category) - - if @layout - - next unless post.layout.name == @layout - - next unless policy(post).show? - %tr - %td - .handle - = image_tag 'arrows-alt-v.svg' - = hidden_field 'post[reorder]', post.sha1, - value: i, class: 'reorder' - %td - %small - = link_to post.layout.name.to_s.humanize, - site_posts_path(@site, layout: post.layout.name) - %br/ - = link_to post.title.value, - site_post_path(@site, post.id) - - if post.attributes.include? :draft - - if post.draft.value - %span.badge.badge-primary - = post_label_t(:draft, post: post) - - if post.attributes.include? :categories - - unless post.categories.value.empty? - %br - %small - - post.categories.value.each do |c| - = link_to c, site_posts_path(@site, category: c) - - %td - = post.date.value.strftime('%F') - %br/ - = post.try(:order).try(:value) - %td - - if policy(post).edit? - = link_to t('posts.edit'), - edit_site_post_path(@site, post.id), - class: 'btn' - - if policy(post).destroy? - = link_to t('posts.destroy'), - site_post_path(@site, post.id), - class: 'btn', - method: :delete, - data: { confirm: t('posts.confirm_destroy') } - - else + - if @posts.empty? %h2= t('posts.none') + - else + = form_tag site_posts_reorder_path, method: :post do + = submit_tag t('posts.reorder'), class: 'btn submit-reorder' + -# TODO: Permitir cambiar el idioma + %table.table.table-condensed.table-draggable + %tbody + - @posts.each_with_index do |post, i| + -# + saltearse el post a menos que esté en la categoría por + la que estamos filtrando + - if @category + - next unless post.attributes.include? :categories + - next unless post.categories.value.include?(@category) + - if @layout + - next unless post.layout.name == @layout + - next unless policy(post).show? + %tr + %td + .handle + = image_tag 'arrows-alt-v.svg' + -# Orden más alto es mayor prioridad + = hidden_field 'post[reorder]', post.sha1, + value: @posts.length - i, class: 'reorder' + %td + %small + = link_to post.layout.name.to_s.humanize, + site_posts_path(@site, layout: post.layout.name) + %br/ + = link_to post.title.value, + site_post_path(@site, post.id) + - if post.attributes.include? :draft + - if post.draft.value + %span.badge.badge-primary + = post_label_t(:draft, post: post) + - if post.attributes.include? :categories + - unless post.categories.value.empty? + %br + %small + - post.categories.value.each do |c| + = link_to c, site_posts_path(@site, category: c) + + %td + = post.date.value.strftime('%F') + %br/ + = post.try(:order).try(:value) + %td + - if policy(post).edit? + = link_to t('posts.edit'), + edit_site_post_path(@site, post.id), + class: 'btn' + - if policy(post).destroy? + = link_to t('posts.destroy'), + site_post_path(@site, post.id), + class: 'btn', + method: :delete, + data: { confirm: t('posts.confirm_destroy') } diff --git a/doc/reordenar.md b/doc/reordenar.md index 99a9ed07..e40e92c8 100644 --- a/doc/reordenar.md +++ b/doc/reordenar.md @@ -23,3 +23,11 @@ otra vez. Lo más controlado sería enviar exactamente el id del post con su nueva ubicación en el orden. Esta es la implementación anterior. + +*** + +El orden es descendiente (fechas más nuevas primero), pero el orden que +estuvimos usando es ascendientes (números más bajos primero). Es más +simple invertir la lógica y hacer todo el orden descendiente. Para eso +los artículos más nuevos tienen que tener el número de orden +correspondiente a la posición en el array ordenado por fecha. diff --git a/test/models/post_test.rb b/test/models/post_test.rb index 827aeb5c..85280722 100644 --- a/test/models/post_test.rb +++ b/test/models/post_test.rb @@ -66,7 +66,7 @@ class PostTest < ActiveSupport::TestCase test 'se pueden guardar sin validar' do assert @post.valid? @post.title.value = '' - assert @post.save(false) + assert @post.save(validate: false) end test 'se pueden guardar los cambios' do