Merge pull request #10869 from demarches-simplifiees/add_id_to_column_second_part

Tech: utilise les objets `SortedColumn`

app/components/dossiers/notified_toggle_component.rb

@@ -3,37 +3,6 @@
 class Dossiers::NotifiedToggleComponent < ApplicationComponent
   def initialize(procedure:, procedure_presentation:)
     @procedure = procedure
-    @procedure_presentation = procedure_presentation
-    @current_sort = procedure_presentation.sort
-  end
-
-  private
-
-  def opposite_order
-    @procedure_presentation.opposite_order_for(current_table, current_column)
-  end
-
-  def active?
-    sorted_by_notifications? && order_desc?
-  end
-
-  def order_desc?
-    current_order == 'desc'
-  end
-
-  def current_order
-    @current_sort['order']
-  end
-
-  def current_table
-    @current_sort['table']
-  end
-
-  def current_column
-    @current_sort['column']
-  end
-
-  def sorted_by_notifications?
-    current_table == 'notifications' && current_column == 'notifications'
+    @sorted_column = procedure_presentation.sorted_column
   end
 end

app/components/dossiers/notified_toggle_component/notified_toggle_component.html.haml

@@ -1,6 +1,9 @@
-= form_tag update_sort_instructeur_procedure_path(procedure_id: @procedure.id, column_id: @procedure.notifications_column.id, order: opposite_order), method: :get, data: { controller: 'autosubmit' } do
+= form_tag update_sort_instructeur_procedure_path(@procedure),
+  method: :get, data: { controller: 'autosubmit' } do
   .fr-fieldset__element.fr-m-0
     .fr-checkbox-group.fr-checkbox-group--sm
-      = check_box_tag :order, opposite_order, active?
-      = label_tag :order, t('.show_notified_first'), class: 'fr-label'
+      = hidden_field_tag 'sorted_column[id]', @procedure.notifications_column.id
+      = hidden_field_tag 'sorted_column[order]', 'asc', id: nil
+      = check_box_tag 'sorted_column[order]', 'desc', @sorted_column.sort_by_notifications?
+      = label_tag 'sorted_column[order]', t('.show_notified_first'), class: 'fr-label'
       = submit_tag t('.show_notified_first'), data: {"checkbox-target": 'submit' }, class: 'visually-hidden'

app/components/instructeurs/column_table_header_component.rb

@@ -1,47 +1,34 @@
 # frozen_string_literal: true
 
 class Instructeurs::ColumnTableHeaderComponent < ApplicationComponent
-  attr_reader :procedure_presentation, :column
-  # maybe extract a ColumnSorter class?
-  #
-
-  def initialize(procedure_presentation:, column:)
-    @procedure_presentation = procedure_presentation
-    @column = column
-  end
-
-  def column_id
-    column.id
-  end
-
-  def sorted_by_current_column?
-    procedure_presentation.sort['table'] == column.table &&
-    procedure_presentation.sort['column'] == column.column
-  end
-
-  def sorted_ascending?
-    current_sort_order == 'asc'
-  end
-
-  def sorted_descending?
-    current_sort_order == 'desc'
-  end
-
-  def aria_sort
-    if sorted_by_current_column?
-      if sorted_ascending?
-        { "aria-sort": "ascending" }
-      elsif sorted_descending?
-        { "aria-sort": "descending" }
-      end
-    else
-      {}
-    end
+  def initialize(procedure_presentation:)
+    @procedure = procedure_presentation.procedure
+    @columns = procedure_presentation.displayed_fields_for_headers
+    @sorted_column = procedure_presentation.sorted_column
   end
 
   private
 
-  def current_sort_order
-    procedure_presentation.sort['order']
+  def update_sort_path(column)
+    id = column.id
+    order = opposite_order_for(column)
+
+    update_sort_instructeur_procedure_path(@procedure, sorted_column: { id:, order: })
+  end
+
+  def opposite_order_for(column)
+    @sorted_column.column == column ? @sorted_column.opposite_order : 'asc'
+  end
+
+  def label_and_arrow(column)
+    return column.label if @sorted_column.column != column
+
+    @sorted_column.ascending? ? "#{column.label} ↑" : "#{column.label} ↓"
+  end
+
+  def aria_sort(column)
+    return {} if @sorted_column.column != column
+
+    @sorted_column.ascending? ? { "aria-sort": "ascending" } : { "aria-sort": "descending" }
   end
 end

app/components/instructeurs/column_table_header_component/column_table_header_component.html.haml

@@ -1,9 +1,3 @@
-%th{ aria_sort, scope: "col", class: column.classname }
-  = link_to update_sort_instructeur_procedure_path(@procedure_presentation.procedure, column_id:, order: @procedure_presentation.opposite_order_for(column.table, column.column)) do
-    - if sorted_by_current_column?
-      - if sorted_ascending?
-        #{column.label} ↑
-      - else
-        #{column.label} ↓
-    - else
-      #{column.label}
+- @columns.each do |column|
+  %th{ aria_sort(column), scope: "col", class: column.classname }
+    = link_to label_and_arrow(column), update_sort_path(column)

app/controllers/instructeurs/procedures_controller.rb

@@ -141,7 +141,7 @@ module Instructeurs
     end
 
     def update_sort
-      procedure_presentation.update_sort(params[:column_id], params[:order])
+      procedure_presentation.update!(sorted_column_params)
 
       redirect_back(fallback_location: instructeur_procedure_url(procedure))
     end
@@ -411,5 +411,9 @@ module Instructeurs
     def cookies_export_key
       "exports_#{@procedure.id}_seen_at"
     end
+
+    def sorted_column_params
+      params.permit(sorted_column: [:order, :id])
+    end
   end
 end

app/models/column.rb

@@ -18,8 +18,13 @@ class Column
     @displayable = displayable
   end
 
+  # the id is a String to be used in forms
   def id = h_id.to_json
+
+  # the h_id is a Hash and hold enough information to find the column
+  # in the ColumnType class, aka be able to do the h_id -> column conversion
   def h_id = { procedure_id: @procedure_id, column_id: "#{table}/#{column}" }
+
   def ==(other) = h_id == other.h_id # using h_id instead of id to avoid inversion of keys
 
   def to_json
@@ -27,4 +32,12 @@ class Column
       table:, column:, label:, classname:, type:, scope:, value_column:, filterable:, displayable:
     }
   end
+
+  def notifications?
+    table == 'notifications' && column == 'notifications'
+  end
+
+  def self.find(h_id)
+    Procedure.with_discarded.find(h_id[:procedure_id]).find_column(h_id:)
+  end
 end

app/models/concerns/columns_concern.rb

@@ -4,17 +4,29 @@ module ColumnsConcern
   extend ActiveSupport::Concern
 
   included do
+    # we cannot use column.id ( == { procedure_id, column_id }.to_json)
+    # as the order of the keys is not guaranteed
+    # instead, we are using h_id == { procedure_id:, column_id: }
+    # another way to find a column is to look for its label
     def find_column(h_id: nil, label: nil)
-      return columns.find { _1.h_id == h_id } if h_id.present?
-      return columns.find { _1.label == label } if label.present?
+      column = columns.find { _1.h_id == h_id } if h_id.present?
+      column = columns.find { _1.label == label } if label.present?
+
+      raise ActiveRecord::RecordNotFound if column.nil?
+
+      column
     end
 
     def columns
-      columns = dossier_columns
-      columns.concat(standard_columns)
-      columns.concat(individual_columns) if for_individual
-      columns.concat(moral_columns) if !for_individual
-      columns.concat(types_de_champ_columns)
+      Current.procedure_columns ||= {}
+
+      Current.procedure_columns[id] ||= begin
+        columns = dossier_columns
+        columns.concat(standard_columns)
+        columns.concat(individual_columns) if for_individual
+        columns.concat(moral_columns) if !for_individual
+        columns.concat(types_de_champ_columns)
+      end
     end
 
     def dossier_id_column
@@ -57,6 +69,10 @@ module ColumnsConcern
       columns
     end
 
+    def default_sorted_column
+      SortedColumn.new(column: notifications_column, order: 'desc')
+    end
+
     private
 
     def standard_columns

app/models/current.rb

@@ -9,4 +9,5 @@ class Current < ActiveSupport::CurrentAttributes
   attribute :no_reply_email
   attribute :request_id
   attribute :user
+  attribute :procedure_columns
 end

app/models/procedure_presentation.rb

@@ -1,11 +1,6 @@
 # frozen_string_literal: true
 
 class ProcedurePresentation < ApplicationRecord
-  EXTRA_SORT_COLUMNS = {
-    'notifications' => ['notifications'],
-    'self' => ['id', 'state']
-  }
-
   TABLE = 'table'
   COLUMN = 'column'
   ORDER = 'order'
@@ -23,13 +18,12 @@ class ProcedurePresentation < ApplicationRecord
   delegate :procedure, :instructeur, to: :assign_to
 
   validate :check_allowed_displayed_fields
-  validate :check_allowed_sort_column
-  validate :check_allowed_sort_order
   validate :check_allowed_filter_columns
   validate :check_filters_max_length
   validate :check_filters_max_integer
 
-  attribute :sorted_column, :jsonb
+  attribute :sorted_column, :sorted_column
+  def sorted_column = super || procedure.default_sorted_column # Dummy override to set default value
 
   attribute :a_suivre_filters, :jsonb, array: true
   attribute :suivis_filters, :jsonb, array: true
@@ -146,37 +140,8 @@ class ProcedurePresentation < ApplicationRecord
       displayed_columns: columns.map(&:h_id)
     )
 
-    if !sort_to_column_id(sort).in?(column_ids)
-      default_column_id = procedure.dossier_id_column.id
-      update_sort(default_column_id, "desc")
-    end
-  end
-
-  def update_sort(column_id, order)
-    h_id = JSON.parse(column_id, symbolize_names: true)
-    column = procedure.find_column(h_id:)
-    order = order.presence || opposite_order_for(column.table, column.column)
-
-    update!(
-      sort: {
-        TABLE => column.table,
-        COLUMN => column.column,
-        ORDER => order
-      },
-      sorted_column: {
-        order:,
-        id: h_id
-      }
-    )
-  end
-
-  def opposite_order_for(table, column)
-    if sort.values_at(TABLE, COLUMN) == [table, column]
-      sort['order'] == 'asc' ? 'desc' : 'asc'
-    elsif [table, column] == ["notifications", "notifications"]
-      'desc' # default order for notifications
-    else
-      'asc'
+    if !sorted_column.column.in?(columns)
+      update(sorted_column: nil)
     end
   end
 
@@ -187,7 +152,9 @@ class ProcedurePresentation < ApplicationRecord
   private
 
   def sorted_ids(dossiers, count)
-    table, column, order = sort.values_at(TABLE, COLUMN, 'order')
+    table = sorted_column.column.table
+    column = sorted_column.column.column
+    order = sorted_column.order
 
     case table
     when 'notifications'
@@ -297,11 +264,6 @@ class ProcedurePresentation < ApplicationRecord
     end.reduce(:&)
   end
 
-  # type_de_champ/4373429
-  def sort_to_column_id(sort)
-    [sort[TABLE], sort[COLUMN]].join(SLASH)
-  end
-
   def find_type_de_champ(column)
     TypeDeChamp
       .joins(:revision_types_de_champ)
@@ -316,17 +278,6 @@ class ProcedurePresentation < ApplicationRecord
     end
   end
 
-  def check_allowed_sort_column
-    check_allowed_field(:sort, sort, EXTRA_SORT_COLUMNS)
-  end
-
-  def check_allowed_sort_order
-    order = sort['order']
-    if !["asc", "desc"].include?(order)
-      errors.add(:sort, "#{order} n’est pas une ordre permis")
-    end
-  end
-
   def check_allowed_filter_columns
     filters.each do |key, columns|
       return true if key == 'migrated'

app/models/sorted_column.rb

@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+class SortedColumn
+  attr_reader :column, :order
+
+  def initialize(column:, order:)
+    @column = column
+    @order = order
+  end
+
+  def ascending? = @order == 'asc'
+
+  def opposite_order = ascending? ? 'desc' : 'asc'
+
+  def ==(other)
+    other&.column == column && other.order == order
+  end
+
+  def sort_by_notifications?
+    @column.notifications? && @order == 'desc'
+  end
+end

app/types/column_type.rb

@@ -0,0 +1,38 @@
+# frozen_string_literal: true
+
+class ColumnType < ActiveRecord::Type::Value
+  # value can come from:
+  # setter: column (Column),
+  # form_input: column.id == { procedure_id:, column_id: }.to_json (String),
+  # from db: { procedure_id:, column_id: } (Hash)
+  def cast(value)
+    case value
+    in NilClass
+      nil
+    in Column
+      value
+    # from form
+    in String => id
+      h_id = JSON.parse(id, symbolize_names: true)
+      Column.find(h_id)
+    # from db
+    in Hash => h_id
+      Column.find(h_id)
+    end
+  end
+
+  # db -> ruby
+  def deserialize(value) = cast(value)
+
+  # ruby -> db
+  def serialize(value)
+    case value
+    in NilClass
+      nil
+    in Column
+      JSON.generate(value.h_id)
+    else
+      raise ArgumentError, "Invalid value for Column serialization: #{value}"
+    end
+  end
+end

app/types/sorted_column_type.rb

@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+class SortedColumnType < ActiveRecord::Type::Value
+  # form_input or setter -> type
+  def cast(value)
+    value = value.deep_symbolize_keys if value.respond_to?(:deep_symbolize_keys)
+
+    case value
+    in SortedColumn
+      value
+    in NilClass # default value
+      nil
+    # from form (id is a string) or from db (id is a hash)
+    in { order: 'asc'|'desc', id: String|Hash } => h
+      SortedColumn.new(column: ColumnType.new.cast(h[:id]), order: h[:order])
+    end
+  end
+
+  # db -> ruby
+  def deserialize(value) = cast(value&.then { JSON.parse(_1) })
+
+  # ruby -> db
+  def serialize(value)
+    case value
+    in NilClass
+      nil
+    in SortedColumn
+      JSON.generate({
+        id: value.column.h_id,
+        order: value.order
+      })
+    else
+      raise ArgumentError, "Invalid value for SortedColumn serialization: #{value}"
+    end
+  end
+end

app/views/instructeurs/procedures/show.html.haml

@@ -93,8 +93,7 @@
                   %th.text-center
                     %input{ type: "checkbox", disabled: @disable_checkbox_all, checked: @disable_checkbox_all, data: { action: "batch-operation#onCheckAll" }, id: dom_id(BatchOperation.new, :checkbox_all), aria: { label: t('views.instructeurs.dossiers.select_all') } }
 
-                - @procedure_presentation.displayed_fields_for_headers.each do |column|
-                  = render Instructeurs::ColumnTableHeaderComponent.new(procedure_presentation: @procedure_presentation, column:)
+                = render Instructeurs::ColumnTableHeaderComponent.new(procedure_presentation: @procedure_presentation)
 
                 %th.follow-col
                   Actions

config/initializers/types.rb

@@ -1,7 +1,11 @@
 # frozen_string_literal: true
 
+require Rails.root.join("app/types/column_type")
 require Rails.root.join("app/types/export_item_type")
+require Rails.root.join("app/types/sorted_column_type")
 
 ActiveSupport.on_load(:active_record) do
+  ActiveRecord::Type.register(:column, ColumnType)
   ActiveRecord::Type.register(:export_item, ExportItemType)
+  ActiveRecord::Type.register(:sorted_column, SortedColumnType)
 end

config/routes.rb

@@ -481,7 +481,7 @@ Rails.application.routes.draw do
         end
 
         patch 'update_displayed_fields'
-        get 'update_sort/:column_id' => 'procedures#update_sort', as: 'update_sort'
+        get 'update_sort' => 'procedures#update_sort', as: 'update_sort'
         post 'add_filter'
         post 'update_filter'
         get 'remove_filter'

spec/controllers/instructeurs/procedures_controller_spec.rb

@@ -886,11 +886,11 @@ describe Instructeurs::ProceduresController, type: :controller do
     end
 
     it 'can change order' do
-      column_id = procedure.find_column(label: "Nom").id
-      expect { get :update_sort, params: { procedure_id: procedure.id, column_id:, order: 'asc' } }
-        .to change { procedure_presentation.sort }
-        .from({ "column" => "notifications", "order" => "desc", "table" => "notifications" })
-        .to({ "column" => "nom", "order" => "asc", "table" => "individual" })
+      column = procedure.find_column(label: "Nom")
+      expect { get :update_sort, params: { procedure_id: procedure.id, sorted_column: { id: column.id, order: 'asc' } } }
+        .to change { procedure_presentation.sorted_column }
+        .from(procedure.default_sorted_column)
+        .to(SortedColumn.new(column:, order: 'asc'))
     end
   end
 

spec/lib/tasks/deployment/20210720133539_remove_migration_status_on_filters_spec.rb

@@ -47,24 +47,4 @@ describe '20201001161931_migrate_filters_to_use_stable_id' do
       expect(procedure_presentation_without_migration.filters['suivis']).to be_present
     end
   end
-
-  context 'when the procedure presentation is invalid' do
-    before do
-      procedure_presentation_with_migration.update_column(
-        :sort,
-       { table: 'invalid-table', column: 'invalid-column', order: 'invalid-order' }
-      )
-    end
-
-    it 'removes the "migrated" key properly' do
-      run_task
-      expect(procedure_presentation_with_migration).not_to be_valid
-      expect(procedure_presentation_with_migration.filters).not_to have_key('migrated')
-    end
-
-    it 'leaves the other keys unchanged' do
-      run_task
-      expect(procedure_presentation_without_migration.filters['suivis']).to be_present
-    end
-  end
 end

spec/lib/tasks/deployment/20240920130741_migrate_procedure_presentation_to_columns.rake_spec.rb

@@ -45,10 +45,10 @@ describe '20240920130741_migrate_procedure_presentation_to_columns.rake' do
 
     order, column_id = procedure_presentation
       .sorted_column
-      .then { |sorted| [sorted['order'], sorted['id']] }
+      .then { |sorted| [sorted.order, sorted.column.h_id] }
 
     expect(order).to eq('desc')
-    expect(column_id).to eq("procedure_id" => procedure_id, "column_id" => "self/en_construction_at")
+    expect(column_id).to eq(procedure_id: procedure_id, column_id: "self/en_construction_at")
 
     expect(procedure_presentation.tous_filters).to eq([])
 

spec/models/concerns/columns_concern_spec.rb

@@ -1,6 +1,22 @@
 # frozen_string_literal: true
 
 describe ColumnsConcern do
+  describe '#find_column' do
+    let(:procedure) { build(:procedure) }
+    let(:notifications_column) { procedure.notifications_column }
+
+    it do
+      label = notifications_column.label
+      expect(procedure.find_column(label:)).to eq(notifications_column)
+
+      h_id = notifications_column.h_id
+      expect(procedure.find_column(h_id:)).to eq(notifications_column)
+
+      unknwon = 'unknown'
+      expect { procedure.find_column(h_id: unknwon) }.to raise_error(ActiveRecord::RecordNotFound)
+    end
+  end
+
   describe "#columns" do
     subject { procedure.columns }
 

spec/models/procedure_presentation_spec.rb

@@ -41,13 +41,6 @@ describe ProcedurePresentation do
       it { expect(build(:procedure_presentation, displayed_fields: [{ table: "user", column: "reset_password_token", "order" => "asc" }])).to be_invalid }
     end
 
-    context 'of sort' do
-      it { expect(build(:procedure_presentation, sort: { table: "notifications", column: "notifications", "order" => "asc" })).to be_valid }
-      it { expect(build(:procedure_presentation, sort: { table: "self", column: "id", "order" => "asc" })).to be_valid }
-      it { expect(build(:procedure_presentation, sort: { table: "self", column: "state", "order" => "asc" })).to be_valid }
-      it { expect(build(:procedure_presentation, sort: { table: "user", column: "reset_password_token", "order" => "asc" })).to be_invalid }
-    end
-
     context 'of filters' do
       it { expect(build(:procedure_presentation, filters: { "suivis" => [{ table: "user", column: "reset_password_token", "order" => "asc" }] })).to be_invalid }
       it { expect(build(:procedure_presentation, filters: { "suivis" => [{ table: "user", column: "email", "value" => "exceedingly long filter value" * 10 }] })).to be_invalid }
@@ -61,19 +54,18 @@ describe ProcedurePresentation do
 
   describe '#sorted_ids' do
     let(:instructeur) { create(:instructeur) }
-    let(:assign_to) { create(:assign_to, procedure: procedure, instructeur: instructeur) }
-    let(:sort) { { 'table' => table, 'column' => column, 'order' => order } }
-    let(:procedure_presentation) { create(:procedure_presentation, assign_to: assign_to, sort: sort) }
+    let(:assign_to) { create(:assign_to, procedure:, instructeur:) }
+    let(:sorted_column) { SortedColumn.new(column:, order:) }
+    let(:procedure_presentation) { create(:procedure_presentation, assign_to:, sorted_column:) }
 
     subject { procedure_presentation.send(:sorted_ids, procedure.dossiers, procedure.dossiers.count) }
 
     context 'for notifications table' do
-      let(:table) { 'notifications' }
-      let(:column) { 'notifications' }
+      let(:column) { procedure.notifications_column }
 
-      let!(:notified_dossier) { create(:dossier, :en_construction, procedure: procedure) }
-      let!(:recent_dossier) { create(:dossier, :en_construction, procedure: procedure) }
-      let!(:older_dossier) { create(:dossier, :en_construction, procedure: procedure) }
+      let!(:notified_dossier) { create(:dossier, :en_construction, procedure:) }
+      let!(:recent_dossier) { create(:dossier, :en_construction, procedure:) }
+      let!(:older_dossier) { create(:dossier, :en_construction, procedure:) }
 
       before do
         notified_dossier.update!(last_champ_updated_at: Time.zone.local(2018, 9, 20))
@@ -96,7 +88,7 @@ describe ProcedurePresentation do
       end
 
       context 'with a dossier terminé' do
-        let!(:notified_dossier) { create(:dossier, :accepte, procedure: procedure) }
+        let!(:notified_dossier) { create(:dossier, :accepte, procedure:) }
         let(:order) { 'desc' }
 
         it { is_expected.to eq([notified_dossier, recent_dossier, older_dossier].map(&:id)) }
@@ -104,11 +96,10 @@ describe ProcedurePresentation do
     end
 
     context 'for self table' do
-      let(:table) { 'self' }
       let(:order) { 'asc' } # Desc works the same, no extra test required
 
       context 'for created_at column' do
-        let(:column) { 'created_at' }
+        let!(:column) { procedure.find_column(label: 'Créé le') }
         let!(:recent_dossier) { Timecop.freeze(Time.zone.local(2018, 10, 17)) { create(:dossier, procedure: procedure) } }
         let!(:older_dossier) { Timecop.freeze(Time.zone.local(2003, 11, 11)) { create(:dossier, procedure: procedure) } }
 
@@ -116,7 +107,7 @@ describe ProcedurePresentation do
       end
 
       context 'for en_construction_at column' do
-        let(:column) { 'en_construction_at' }
+        let!(:column) { procedure.find_column(label: 'En construction le') }
         let!(:recent_dossier) { create(:dossier, :en_construction, procedure: procedure, en_construction_at: Time.zone.local(2018, 10, 17)) }
         let!(:older_dossier) { create(:dossier, :en_construction, procedure: procedure, en_construction_at: Time.zone.local(2013, 1, 1)) }
 
@@ -124,7 +115,7 @@ describe ProcedurePresentation do
       end
 
       context 'for updated_at column' do
-        let(:column) { 'updated_at' }
+        let(:column) { procedure.find_column(label: 'Mis à jour le') }
         let(:recent_dossier) { create(:dossier, procedure: procedure) }
         let(:older_dossier) { create(:dossier, procedure: procedure) }
 
@@ -140,10 +131,10 @@ describe ProcedurePresentation do
     context 'for type_de_champ table' do
       context 'with no revisions' do
         let(:table) { 'type_de_champ' }
-        let(:column) { procedure.active_revision.types_de_champ_public.first.stable_id.to_s }
+        let(:column) { procedure.find_column(label: first_type_de_champ.libelle) }
 
-        let(:beurre_dossier) { create(:dossier, procedure: procedure) }
-        let(:tartine_dossier) { create(:dossier, procedure: procedure) }
+        let(:beurre_dossier) { create(:dossier, procedure:) }
+        let(:tartine_dossier) { create(:dossier, procedure:) }
 
         before do
           beurre_dossier.project_champs_public.first.update(value: 'beurre')
@@ -165,12 +156,11 @@ describe ProcedurePresentation do
 
       context 'with a revision adding a new type_de_champ' do
         let!(:tdc) { { type_champ: :text, libelle: 'nouveau champ' } }
-        let(:table) { 'type_de_champ' }
-        let(:column) { procedure.active_revision.types_de_champ_public.last.stable_id.to_s }
+        let(:column) { procedure.find_column(label: 'nouveau champ') }
 
-        let(:nothing_dossier) { create(:dossier, procedure: procedure) }
-        let(:beurre_dossier) { create(:dossier, procedure: procedure) }
-        let(:tartine_dossier) { create(:dossier, procedure: procedure) }
+        let!(:nothing_dossier) { create(:dossier, procedure:) }
+        let!(:beurre_dossier) { create(:dossier, procedure:) }
+        let!(:tartine_dossier) { create(:dossier, procedure:) }
 
         before do
           nothing_dossier
@@ -182,20 +172,19 @@ describe ProcedurePresentation do
 
         context 'asc' do
           let(:order) { 'asc' }
-          it { is_expected.to eq([beurre_dossier, tartine_dossier, nothing_dossier].map(&:id)) }
+          it { is_expected.to eq([nothing_dossier, beurre_dossier, tartine_dossier].map(&:id)) }
         end
 
         context 'desc' do
           let(:order) { 'desc' }
-          it { is_expected.to eq([nothing_dossier, tartine_dossier, beurre_dossier].map(&:id)) }
+          it { is_expected.to eq([tartine_dossier, beurre_dossier, nothing_dossier].map(&:id)) }
         end
       end
     end
 
     context 'for type_de_champ_private table' do
       context 'with no revisions' do
-        let(:table) { 'type_de_champ' }
-        let(:column) { procedure.active_revision.types_de_champ_private.first.stable_id.to_s }
+        let(:column) { procedure.find_column(label: procedure.active_revision.types_de_champ_private.first.libelle) }
 
         let(:biere_dossier) { create(:dossier, procedure: procedure) }
         let(:vin_dossier) { create(:dossier, procedure: procedure) }
@@ -217,38 +206,9 @@ describe ProcedurePresentation do
           it { is_expected.to eq([vin_dossier, biere_dossier].map(&:id)) }
         end
       end
-
-      context 'with a revision adding a new type_de_champ' do
-        let!(:tdc) { { type_champ: :text, private: true, libelle: 'nouveau champ' } }
-        let(:table) { 'type_de_champ' }
-        let(:column) { procedure.active_revision.types_de_champ_private.last.stable_id.to_s }
-
-        let(:nothing_dossier) { create(:dossier, procedure: procedure) }
-        let(:biere_dossier) { create(:dossier, procedure: procedure) }
-        let(:vin_dossier) { create(:dossier, procedure: procedure) }
-
-        before do
-          nothing_dossier
-          procedure.draft_revision.add_type_de_champ(tdc)
-          procedure.publish_revision!
-          biere_dossier.project_champs_private.last.update(value: 'biere')
-          vin_dossier.project_champs_private.last.update(value: 'vin')
-        end
-
-        context 'asc' do
-          let(:order) { 'asc' }
-          it { is_expected.to eq([biere_dossier, vin_dossier, nothing_dossier].map(&:id)) }
-        end
-
-        context 'desc' do
-          let(:order) { 'desc' }
-          it { is_expected.to eq([nothing_dossier, vin_dossier, biere_dossier].map(&:id)) }
-        end
-      end
     end
 
     context 'for individual table' do
-      let(:table) { 'individual' }
       let(:order) { 'asc' } # Desc works the same, no extra test required
 
       let(:procedure) { create(:procedure, :for_individual) }
@@ -257,26 +217,25 @@ describe ProcedurePresentation do
       let!(:last_dossier) { create(:dossier, procedure: procedure, individual: build(:individual, gender: 'Mme', prenom: 'Zora', nom: 'Zemmour')) }
 
       context 'for gender column' do
-        let(:column) { 'gender' }
+        let(:column) { procedure.find_column(label: 'Civilité') }
 
         it { is_expected.to eq([first_dossier, last_dossier].map(&:id)) }
       end
 
       context 'for prenom column' do
-        let(:column) { 'prenom' }
+        let(:column) { procedure.find_column(label: 'Prénom') }
 
         it { is_expected.to eq([first_dossier, last_dossier].map(&:id)) }
       end
 
       context 'for nom column' do
-        let(:column) { 'nom' }
+        let(:column) { procedure.find_column(label: 'Nom') }
 
         it { is_expected.to eq([first_dossier, last_dossier].map(&:id)) }
       end
     end
 
     context 'for followers_instructeurs table' do
-      let(:table) { 'followers_instructeurs' }
       let(:order) { 'asc' } # Desc works the same, no extra test required
 
       let!(:dossier_z) { create(:dossier, :en_construction, procedure: procedure) }
@@ -290,15 +249,14 @@ describe ProcedurePresentation do
       end
 
       context 'for email column' do
-        let(:column) { 'email' }
+        let(:column) { procedure.find_column(label: 'Email instructeur') }
 
         it { is_expected.to eq([dossier_a, dossier_z, dossier_without_instructeur].map(&:id)) }
       end
     end
 
     context 'for avis table' do
-      let(:table) { 'avis' }
-      let(:column) { 'question_answer' }
+      let(:column) { procedure.find_column(label: 'Avis oui/non') }
       let(:order) { 'asc' }
 
       let!(:dossier_yes) { create(:dossier, procedure:) }
@@ -315,8 +273,7 @@ describe ProcedurePresentation do
 
     context 'for other tables' do
       # All other columns and tables work the same so it’s ok to test only one
-      let(:table) { 'etablissement' }
-      let(:column) { 'code_postal' }
+      let(:column) { procedure.find_column(label: 'Code postal') }
       let(:order) { 'asc' } # Desc works the same, no extra test required
 
       let!(:huitieme_dossier) { create(:dossier, procedure: procedure, etablissement: create(:etablissement, code_postal: '75008')) }
@@ -971,7 +928,7 @@ describe ProcedurePresentation do
   describe '#update_displayed_fields' do
     let(:procedure_presentation) do
       create(:procedure_presentation, assign_to:).tap do |pp|
-        pp.update_sort(procedure.find_column(label: 'Demandeur').id, 'desc')
+        pp.update(sorted_column: SortedColumn.new(column: procedure.find_column(label: 'Demandeur'), order: 'desc'))
       end
     end
 
@@ -992,26 +949,8 @@ describe ProcedurePresentation do
         { "column_id" => "self/updated_at", "procedure_id" => procedure.id }
       ])
 
-      expect(procedure_presentation.sorted_column['id']).to eq("column_id" => "self/id", "procedure_id" => procedure.id)
-      expect(procedure_presentation.sorted_column['order']).to eq('desc')
-    end
-  end
-
-  describe '#update_sort' do
-    let(:procedure_presentation) { create(:procedure_presentation, assign_to:) }
-
-    subject do
-      column_id = procedure.find_column(label: 'En construction le').id
-      procedure_presentation.update_sort(column_id, 'asc')
-    end
-
-    it 'should update sort and order' do
-      expect(procedure_presentation.sorted_column).to be_nil
-
-      subject
-
-      expect(procedure_presentation.sorted_column['id']).to eq("column_id" => "self/en_construction_at", "procedure_id" => procedure.id)
-      expect(procedure_presentation.sorted_column['order']).to eq('asc')
+      expect(procedure_presentation.sorted_column).to eq(procedure.default_sorted_column)
+      expect(procedure_presentation.sorted_column.order).to eq('desc')
     end
   end
 end

spec/models/sorted_column_spec.rb

@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+describe SortedColumn do
+  let(:column) { Column.new(procedure_id: 1, table: 'table', column: 'column') }
+  let(:sorted_column) { SortedColumn.new(column: column, order: 'asc') }
+
+  describe '==' do
+    it 'returns true for the same sorted column' do
+      other = SortedColumn.new(column: column, order: 'asc')
+      expect(sorted_column == other).to eq(true)
+    end
+
+    it 'returns false if the order is different' do
+      other = SortedColumn.new(column: column, order: 'desc')
+      expect(sorted_column == other).to eq(false)
+    end
+
+    it 'returns false if the column is different' do
+      other_column = Column.new(procedure_id: 1, table: 'table', column: 'other')
+      other = SortedColumn.new(column: other_column, order: 'asc')
+      expect(sorted_column == other).to eq(false)
+    end
+
+    it 'returns false if the other is nil' do
+      expect(sorted_column == nil).to eq(false)
+    end
+  end
+end

spec/types/column_type_spec.rb

@@ -0,0 +1,71 @@
+# frozen_string_literal: true
+
+describe ColumnType do
+  let(:type) { ColumnType.new }
+
+  describe 'cast' do
+    it 'from Column' do
+      column = Column.new(procedure_id: 1, table: 'table', column: 'column')
+      expect(type.cast(column)).to eq(column)
+    end
+
+    it 'from nil' do
+      expect(type.cast(nil)).to eq(nil)
+    end
+
+    describe 'from form' do
+      it 'with valid column id' do
+        column = Column.new(procedure_id: 1, table: 'table', column: 'column')
+
+        expect(Column).to receive(:find).with(column.h_id).and_return(column)
+        expect(type.cast(column.id)).to eq(column)
+      end
+
+      it 'with invalid column id' do
+        expect { type.cast('invalid') }.to raise_error(JSON::ParserError)
+
+        id = { procedure_id: 'invalid', column_id: 'nop' }.to_json
+        expect { type.cast(id) }.to raise_error(ActiveRecord::RecordNotFound)
+      end
+    end
+
+    describe 'from db' do
+      it 'with valid column id' do
+        column = Column.new(procedure_id: 1, table: 'table', column: 'column')
+        expect(Column).to receive(:find).with(column.h_id).and_return(column)
+        expect(type.cast(column.h_id)).to eq(column)
+      end
+
+      it 'with invalid column id' do
+        h_id = { procedure_id: 'invalid', column_id: 'nop' }
+        expect { type.cast(h_id) }.to raise_error(ActiveRecord::RecordNotFound)
+      end
+    end
+  end
+
+  describe 'deserialize' do
+    context 'with valid value' do
+      it 'works' do
+        column = Column.new(procedure_id: 1, table: 'table', column: 'column')
+        expect(Column).to receive(:find).with(column.h_id).and_return(column)
+
+        expect(type.deserialize(column.h_id)).to eq(column)
+      end
+    end
+  end
+
+  describe 'serialize' do
+    it 'with SortedColumn' do
+      column = Column.new(procedure_id: 1, table: 'table', column: 'column')
+      expect(type.serialize(column)).to eq(column.h_id.to_json)
+    end
+
+    it 'with nil' do
+      expect(type.serialize(nil)).to eq(nil)
+    end
+
+    it 'with invalid value' do
+      expect { type.serialize('invalid') }.to raise_error(ArgumentError)
+    end
+  end
+end

spec/types/sorted_column_type_spec.rb

@@ -0,0 +1,71 @@
+# frozen_string_literal: true
+
+describe SortedColumnType do
+  let(:type) { SortedColumnType.new }
+
+  describe 'cast' do
+    it 'from SortedColumn' do
+      column = Column.new(procedure_id: 1, table: 'table', column: 'column')
+      sorted_column = SortedColumn.new(column:, order: 'asc')
+      expect(type.cast(sorted_column)).to eq(sorted_column)
+    end
+
+    it 'from nil' do
+      expect(type.cast(nil)).to eq(nil)
+    end
+
+    describe 'from form' do
+      it 'with valid column id' do
+        column = Column.new(procedure_id: 1, table: 'table', column: 'column')
+        h = { order: 'asc', id: column.id }
+
+        expect(Column).to receive(:find).with(column.h_id).and_return(column)
+        expect(type.cast(h)).to eq(SortedColumn.new(column: column, order: 'asc'))
+      end
+
+      it 'with invalid column id' do
+        h = { order: 'asc', id: 'invalid' }
+        expect { type.cast(h) }.to raise_error(JSON::ParserError)
+
+        h = { order: 'asc', id: { procedure_id: 'invalid', column_id: 'nop' }.to_json }
+        expect { type.cast(h) }.to raise_error(ActiveRecord::RecordNotFound)
+      end
+
+      it 'with invalid order' do
+        column = Column.new(procedure_id: 1, table: 'table', column: 'column')
+        h = { order: 'invalid', id: column.id }
+        expect { type.cast(h) }.to raise_error(NoMatchingPatternError)
+      end
+    end
+  end
+
+  describe 'deserialize' do
+    context 'with valid value' do
+      it 'works' do
+        column = Column.new(procedure_id: 1, table: 'table', column: 'column')
+        expect(Column).to receive(:find).with(column.h_id).and_return(column)
+        expect(type.deserialize({ id: column.h_id, order: 'asc' }.to_json)).to eq(SortedColumn.new(column: column, order: 'asc'))
+      end
+    end
+
+    context 'with nil' do
+      it { expect(type.deserialize(nil)).to eq(nil) }
+    end
+  end
+
+  describe 'serialize' do
+    it 'with SortedColumn' do
+      column = Column.new(procedure_id: 1, table: 'table', column: 'column')
+      sorted_column = SortedColumn.new(column: column, order: 'asc')
+      expect(type.serialize(sorted_column)).to eq({ id: column.h_id, order: 'asc' }.to_json)
+    end
+
+    it 'with nil' do
+      expect(type.serialize(nil)).to eq(nil)
+    end
+
+    it 'with invalid value' do
+      expect { type.serialize('invalid') }.to raise_error(ArgumentError)
+    end
+  end
+end