cambiar el algoritmo de ordenamiento

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.
2024-11-23 23:06:24 +00:00 · 2019-12-11 17:05:31 -03:00 · 2019-12-11 17:05:31 -03:00 · 0c81809edf
commit 0c81809edf
parent 57457cfcc8
11 changed files with 105 additions and 82 deletions
--- 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
--- 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'));
  });
 })
--- 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
--- 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
--- 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
--- 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
--- 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

--- 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
--- 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') }
--- 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.
--- 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