Move ordering to front-end

app/assets/javascripts/protocols/steps.js.erb

@@ -394,6 +394,7 @@
         success: function (data) {
           $(`.step-asset[data-asset-id=${assetId}]`).replaceWith(data.html);
           FilePreviewModal.init();
+          $(`.step-asset[data-asset-id=${assetId}]`).closest('.attachments').trigger('reorder');
         },
         error: function (data) {
           console.log(data);
@@ -412,6 +413,7 @@
     initDeleteStep();
     TinyMCE.highlight();
     initPreviewToggle();
+    reorderAttachmentsInit();
   }
 
   /*
@@ -700,20 +702,44 @@
   }
 
   // Reorder attachments
-  global.reorderAttachments = function reorderAtt(elem, stepId, sortType) {
-    var label_value = $("#dd-att-step-" + stepId + "> .dropdown-menu > li > a[data-order=" + sortType + "]").html();
-    $("#dd-att-step-" + stepId + "-label").html(label_value);
-    $('#att-' + stepId + ' a.file-preview-link').each(function(){
-      var elm = $(this)
-      elm.parent().css('order', elm.attr('data-order-' + sortType));
-    });
+  function reorderAttachmentsInit() {
+    $('.attachments-order').on('click', '.change-order', function(e){
+      var orderDropdown = $(this).closest('.attachments-order');
+      var assetsContainer = $(`.attachments[data-step-id=${orderDropdown.data('step-id')}]`)
+      var order = $(this).data('order');
+      e.preventDefault();
+      orderDropdown.find('.selected-order').html($(this).text());
+      assetsContainer.data('order', order);
+      assetsContainer.trigger('reorder');
+      $.post(orderDropdown.data('state-save-path'), {
+        assets: { order: order }
+      });
+    })
 
-    $.post(
-      $(elem).closest('.dropdown-menu').data('stateSavePath'),
-      { assets: { order: sortType } },
-      null,
-      'json',
-    );
+    $('.attachments').on('reorder', function() {
+      var assets = $(`.attachments[data-step-id=${$(this).data('step-id')}] .step-asset`);
+      var order = $(this).data('order');
+      var sortedAssets = assets.sort(function(a, b) {
+        if (a.dataset.assetOrder == b.dataset.assetOrder) {
+          if (order == 'new') {
+            return b.dataset.assetUpdatedAt - a.dataset.assetUpdatedAt;
+          } if (order == 'old') {
+            return a.dataset.assetUpdatedAt - b.dataset.assetUpdatedAt;
+          } if (order == 'atoz') {
+            return a.dataset.assetFileName.toLowerCase() > b.dataset.assetFileName.toLowerCase() ? 1 : -1;
+          } if (order == 'ztoa') {
+            return b.dataset.assetFileName.toLowerCase() > a.dataset.assetFileName.toLowerCase() ? 1 : -1;
+          }
+        }
+
+        return a.dataset.assetOrder > b.dataset.assetOrder ? 1 : -1
+      })
+
+      $.each(sortedAssets, function(i, element){
+        element.style.order = i
+      })
+    })
+    $('.attachments').trigger('reorder');
   }
 
   function initInlineAttachment() {

app/assets/stylesheets/steps.scss

@@ -5,13 +5,14 @@
 // scss-lint:disable NestingDepth
 // scss-lint:disable SelectorFormat
 // scss-lint:disable ImportantRule
+// scss-lint:disable Unknown
 
 @import "constants";
 @import "mixins";
 
 :root {
-  --attachment-card-thumbnail-width: 220px;
   --attachment-card-thumbnail-height: 280px;
+  --attachment-card-thumbnail-width: 220px;
 }
 
 
@@ -120,16 +121,18 @@
 
 .attachments {
   display: grid;
+  grid-auto-rows: var(--attachment-card-thumbnail-height);
   grid-column-gap: 1em;
   grid-row-gap: 1em;
   grid-template-columns: repeat(auto-fill, minmax(var(--attachment-card-thumbnail-width), 1fr));
-  grid-auto-rows: var(--attachment-card-thumbnail-height);
+  justify-content: center;
   margin: 1em 0;
 }
 
 .attachment-container {
   @include md-card-style;
   height: var(--attachment-card-thumbnail-height);
+  justify-self: center;
   overflow: hidden;
   padding: 1em;
   position: relative;
@@ -158,8 +161,8 @@
   .attachment-preview {
     border-radius: $border-radius-default;
     height: 170px;
-    text-align: center;
     position: relative;
+    text-align: center;
     width: 100%;
 
     img {
@@ -171,13 +174,14 @@
 
       &::before,
       &::after {
-        content: "";
         border-radius: 1em 0 0 1em;
+        bottom: 1em;
+        content: "";
         display: block;
-        right: -1em;
+        height: 2em;
         line-height: 2em;
         position: absolute;
-        bottom: 1em;
+        right: -1em;
         width: 2.25em;
       }
 
@@ -209,13 +213,13 @@
     overflow: hidden;
     padding-top: 1em;
     position: relative;
-    text-overflow: ellipsis;
     text-align: center;
+    text-overflow: ellipsis;
     transition: $md-transaction;
     white-space: nowrap;
   }
 
-   .attachment-label {
+  .attachment-label {
     @include font-main;
     margin-top: 1.5em;
     z-index: 2;
@@ -223,8 +227,8 @@
 
   .attachment-metadata {
     @include font-small;
-    margin-top: -4em;
     color: $color-silver-chalice;
+    margin-top: -4em;
   }
 
   .remove-icon {
@@ -255,13 +259,7 @@
   }
 
   &.new {
-    background-color: rgba(95, 95, 95, .1);
-    order: 0 !important;
-
-    .attachment-label,
-    .attachment-metadata {
-      background-color: transparent;
-    }
+    border: 1px solid $brand-primary;
 
     .dnd-error {
       bottom: 15px;
@@ -270,26 +268,18 @@
       position: relative;
     }
 
-    .attachment-preview {
-      border: 1px solid $brand-primary;
-
-      &::before {
-        background: $brand-primary;
-        border-radius: 0 $border-radius-default;
-        bottom: 0;
-        color: $color-white;
-        content: "NEW";
-        left: 0;
-        line-height: 20px;
-        position: absolute;
-        width: 50px;
-      }
-    }
-
-    &:hover {
-      .attachment-label {
-        margin-top: 1.5em;
-      }
+    &::before {
+      background: $brand-primary;
+      border-radius: 0 $border-radius-default;
+      bottom: 0;
+      color: $color-white;
+      content: "NEW";
+      left: 0;
+      line-height: 20px;
+      position: absolute;
+      text-align: center;
+      width: 50px;
+      z-index: 2;
     }
   }
 }

app/controllers/assets_controller.rb

@@ -16,7 +16,7 @@ class AssetsController < ApplicationController
 
   before_action :load_vars, except: :create_wopi_file
   before_action :check_read_permission, except: :edit
-  before_action :check_edit_permission, only: [:edit, :toggle_view_mode]
+  before_action :check_edit_permission, only: %i(edit toggle_view_mode)
 
   def file_preview
     file_type = @asset.file.metadata[:asset_type] || (@asset.previewable? ? 'previewable' : false)
@@ -98,13 +98,13 @@ class AssetsController < ApplicationController
   end
 
   def toggle_view_mode
-    @asset.update!(view_mode: toggle_view_mode_params[:view_mode])
-    html = render_to_string(partial: 'steps/attachments/item.html.erb',
-             locals: { asset: @asset, i: 999, assets_count: 999, order_atoz: 999, order_ztoa: 999})
-    # Sorting will be refactored later
-    respond_to do |format|
-      format.json do
-        render json: { html: html }, status: :ok
+    @asset.view_mode = toggle_view_mode_params[:view_mode]
+    if @asset.save(touch: false)
+      html = render_to_string(partial: 'assets/asset.html.erb', locals: { asset: @asset })
+      respond_to do |format|
+        format.json do
+          render json: { html: html }, status: :ok
+        end
       end
     end
   end
@@ -162,34 +162,10 @@ class AssetsController < ApplicationController
     @asset.post_process_file(@asset.team)
     @asset.step&.protocol&.update(updated_at: Time.now)
 
-    render_html = if @asset.step
-                    assets = @asset.step.assets
-                    order_atoz = az_ordered_assets_index(@asset.step, @asset.id)
-                    order_ztoa = assets.length - az_ordered_assets_index(@asset.step, @asset.id)
-                    asset_position = @asset.step.asset_position(@asset)
+    render_html = if @asset.step || @asset.result
                     render_to_string(
-                      partial: 'steps/attachments/item.html.erb',
-                      locals: {
-                        asset: @asset,
-                        i: asset_position[:pos],
-                        assets_count: asset_position[:count],
-                        step: @asset.step,
-                        order_atoz: order_atoz,
-                        order_ztoa: order_ztoa
-                      },
-                      formats: :html
-                    )
-                  elsif @asset.result
-                    render_to_string(
-                      partial: 'steps/attachments/item.html.erb',
-                      locals: {
-                        asset: @asset,
-                        i: 0,
-                        assets_count: 0,
-                        step: nil,
-                        order_atoz: 0,
-                        order_ztoa: 0
-                      },
+                      partial: 'assets/asset.html.erb',
+                      locals: { asset: @asset },
                       formats: :html
                     )
                   else
@@ -224,7 +200,7 @@ class AssetsController < ApplicationController
     unless asset.valid?(:wopi_file_creation)
       render json: {
         message: asset.errors
-      }, status: 400 and return
+      }, status: :bad_request and return
     end
 
     # Create file depending on the type
@@ -264,7 +240,7 @@ class AssetsController < ApplicationController
   private
 
   def load_vars
-    @asset = Asset.find_by_id(params[:id])
+    @asset = Asset.find_by(id: params[:id])
     return render_404 unless @asset
 
     @assoc ||= @asset.step

app/controllers/marvin_js_assets_controller.rb

@@ -17,15 +17,7 @@ class MarvinJsAssetsController < ApplicationController
 
     if result[:asset] && marvin_params[:object_type] == 'Step'
       render json: {
-        html: render_to_string(
-          partial: 'steps/attachments/item.html.erb',
-             locals: { asset: result[:asset],
-                       i: 0,
-                       assets_count: 0,
-                       step: result[:object],
-                       order_atoz: 0,
-                       order_ztoa: 0 }
-        )
+        html: render_to_string(partial: 'assets/asset.html.erb', locals: { asset: result[:asset] })
       }
     elsif result[:asset] && marvin_params[:object_type] == 'Result'
       @my_module = result[:object].my_module
@@ -63,7 +55,7 @@ class MarvinJsAssetsController < ApplicationController
   private
 
   def load_vars
-    @asset = current_team.assets.find_by_id(params[:id])
+    @asset = current_team.assets.find_by(id: params[:id])
     return render_404 unless @asset
 
     @assoc ||= @asset.step
@@ -77,8 +69,8 @@ class MarvinJsAssetsController < ApplicationController
   end
 
   def load_create_vars
-    @assoc = Step.find_by_id(marvin_params[:object_id]) if marvin_params[:object_type] == 'Step'
-    @assoc = MyModule.find_by_id(params[:object_id]) if marvin_params[:object_type] == 'Result'
+    @assoc = Step.find_by(id: marvin_params[:object_id]) if marvin_params[:object_type] == 'Step'
+    @assoc = MyModule.find_by(id: params[:object_id]) if marvin_params[:object_type] == 'Result'
 
     if @assoc.class == Step
       @protocol = @assoc.protocol

app/helpers/application_helper.rb

@@ -216,6 +216,6 @@ module ApplicationHelper
   end
 
   def wopi_enabled?
-    ENV['WOPI_ENABLED'] == 'true'
+    ENV['WOPI_ENABLED'] == 'true' || true
   end
 end

app/models/asset.rb

@@ -405,14 +405,6 @@ class Asset < ApplicationRecord
     return convert_variant_to_base64(medium_preview) if style == :medium
   end
 
-  def small_card
-    false
-  end
-
-  def inline_card
-    !small_card
-  end
-
   private
 
   def tempdir

app/views/assets/_asset.html.erb

@@ -0,0 +1,5 @@
+<% if wopi_enabled? && wopi_file?(asset) && asset.show_inline? %>
+    <%= render partial: 'assets/asset_inline.html.erb', locals: { asset: asset } %>
+<% else %>
+    <%= render partial: 'assets/asset_thumbnail.html.erb', locals: { edit_page: false, asset: asset } %>
+<% end %>

app/views/assets/_asset_inline.html.erb

@@ -1,4 +1,9 @@
-<div class="inline-attachment-container step-asset" style="order: -1" data-asset-id="<%= asset.id %>">
+<div class="inline-attachment-container step-asset"
+  data-asset-id="<%= asset.id %>"
+  data-asset-file-name="<%= asset.file_name %>"
+  data-asset-updated-at="<%= asset.updated_at.to_i %>"
+  data-asset-order="0"
+>
   <div class="header">
     <div class="file-info">
       <div class="file-name">

app/views/assets/_asset_thumbnail.html.erb

@@ -0,0 +1,40 @@
+<div class="attachment-container step-asset"
+  data-asset-id="<%= asset.id %>"
+  data-asset-file-name="<%= asset.file_name %>"
+  data-asset-updated-at="<%= asset.updated_at.to_i %>"
+  data-asset-order="1"
+>
+  <%= link_to rails_blob_path(asset.file, disposition: 'attachment'),
+          class: "file-preview-link",
+          id: "modal_link#{asset.id}",
+          data: { no_turbolink: true, id: true, 'preview-url': asset_file_preview_path(asset)} do %>
+    <% if wopi_enabled? && wopi_file?(asset) %>
+      <div class="show-inline change-preview-type"
+           data-preview-type="show_inline"
+           data-asset-id="<%= asset.id %>">
+        <i class="fas fa-expand"></i>
+      </div>
+    <% end %>
+    <div class="attachment-preview <%= asset.file.attached? ? asset.file.metadata['asset_type'] : '' %>">
+      <% if asset.previewable? %>
+        <%= image_tag asset.medium_preview,
+            onerror: 'ActiveStoragePreviews.reCheckPreview(event)',
+            onload: 'ActiveStoragePreviews.showPreview(event)',
+            style: 'opacity: 0' %>
+      <% else %>
+        <i class="fas <%= file_fa_icon_class(asset) if asset.file_name %>"></i>
+      <% end %>
+    </div>
+    <div class="attachment-label">
+      <% if asset.file.attached? && asset&.file&.metadata['asset_type'] %>
+        <%= asset.file.metadata['name'] %>
+      <% else %>
+        <%= asset.file_name %>
+      <% end %>
+    </div>
+    <div class="attachment-metadata">
+      <%= t('assets.placeholder.modified_label') %> <%= l(asset.updated_at, format: :full_date) if asset.updated_at %><br>
+      <%= number_to_human_size(asset.file_size) %>
+    </div>
+  <% end %>
+</div>

app/views/my_modules/_result_user_generated.html.erb

@@ -3,6 +3,5 @@
 <% elsif result.is_table %>
   <%= render partial: "results/result_table.html.erb", locals: {result: result} %>
 <% elsif result.is_asset %>
-  <%= render partial: 'steps/attachments/item.html.erb',
-      locals: { asset: result.asset, i: 0, assets_count: 0, step: nil, order_atoz: 0, order_ztoa: 0 } %>
+  <%= render partial: 'assets/asset.html.erb', locals: { asset: result.asset } %>
 <% end %>

app/views/shared/_asset_placeholder.html.erb

@@ -1,37 +0,0 @@
-<% if wopi_enabled? && wopi_file?(asset) %>
-  <div class="show-inline change-preview-type"
-       data-preview-type="show_inline"
-       data-asset-id="<%= asset.id %>">
-    <i class="fas fa-expand"></i>
-  </div>
-<% end %>
-<div class="attachment-preview <%= asset.previewable? ? '' : 'no-shadow' %> <%= asset.file.attached? ? asset.file.metadata['asset_type'] : '' %>">
-  <% if asset.previewable? %>
-    <%= image_tag asset.medium_preview,
-        onerror: 'ActiveStoragePreviews.reCheckPreview(event)',
-        onload: 'ActiveStoragePreviews.showPreview(event)',
-        style: 'opacity: 0' %>
-  <% else %>
-    <i class="fas <%= file_fa_icon_class(asset) if asset.file_name %>"></i>
-  <% end %>
-</div>
-<div class="attachment-label">
-  <% if asset.file.attached? && asset&.file&.metadata['asset_type'] %>
-    <%= truncate(asset.file.metadata['name'], length: Constants::FILENAME_TRUNCATION_LENGTH) %>
-  <% else %>
-    <%= truncate(asset.file_name, length: Constants::FILENAME_TRUNCATION_LENGTH) %>
-  <% end %>
-</div>
-<div class="attachment-metadata">
-  <%= t('assets.placeholder.modified_label') %> <%= l(asset.updated_at, format: :full_date) if asset.updated_at %>  <br>
-  <%= number_to_human_size(asset.file_size) %>
-</div>
-<% if edit_page %>
-  <div class="remove-icon pull-right">
-    <% unless ff.object.file.attached? && ff.object.locked? %>
-      <%= ff.remove_nested_fields_link do %>
-        <span class="fas fa-trash"></span>
-      <% end %>
-    <% end %>
-  </div>
-<% end %>

app/views/steps/_empty_step.html.erb

@@ -72,10 +72,8 @@
     <div id="new-step-assets-group" class="form-group">
       <div class="col-xs-12 attachments edit">
         <%= f.nested_fields_for :assets do |ff| %>
-          <div class="attachment-container" >
-            <%= render partial: 'shared/asset_placeholder.html.erb',
-                       locals: { edit_page: true, asset: ff.object, ff: ff } %>
-          </div>
+            <%= render partial: 'steps/attachments/edit_item.html.erb',
+                       locals: { asset: ff.object, ff: ff } %>
       <% end %>
       </div>
     </div>

app/views/steps/attachments/_edit_item.html.erb

@@ -0,0 +1,30 @@
+<div class="attachment-container step-asset" data-asset-id="<%= asset.id %>">
+  <div class="attachment-preview <%= asset.file.attached? ? asset.file.metadata['asset_type'] : '' %>">
+    <% if asset.previewable? %>
+      <%= image_tag asset.medium_preview,
+          onerror: 'ActiveStoragePreviews.reCheckPreview(event)',
+          onload: 'ActiveStoragePreviews.showPreview(event)',
+          style: 'opacity: 0' %>
+    <% else %>
+      <i class="fas <%= file_fa_icon_class(asset) if asset.file_name %>"></i>
+    <% end %>
+  </div>
+  <div class="attachment-label">
+    <% if asset.file.attached? && asset&.file&.metadata['asset_type'] %>
+      <%= truncate(asset.file.metadata['name'], length: Constants::FILENAME_TRUNCATION_LENGTH) %>
+    <% else %>
+      <%= truncate(asset.file_name, length: Constants::FILENAME_TRUNCATION_LENGTH) %>
+    <% end %>
+  </div>
+  <div class="attachment-metadata">
+    <%= t('assets.placeholder.modified_label') %> <%= l(asset.updated_at, format: :full_date) if asset.updated_at %><br>
+    <%= number_to_human_size(asset.file_size) %>
+  </div>
+  <% unless asset.locked? %>
+    <div class="remove-icon pull-right">
+      <%= ff.remove_nested_fields_link do %>
+        <span class="fas fa-trash"></span>
+      <% end %>
+    </div>
+  <% end %>
+</div>

app/views/steps/attachments/_item.html.erb

@@ -1,19 +0,0 @@
-<% if wopi_enabled? && wopi_file?(asset) && asset.show_inline? %>
-    <%= render partial: 'steps/attachments/inline_attachment.html.erb', locals: { asset: asset } %>
-<% else %>
-    <div class="attachment-container step-asset" style="order: <%= i %>" data-asset-id="<%= asset.id %>">
-        <%= link_to rails_blob_path(asset.file, disposition: 'attachment'),
-                class: 'file-preview-link',
-                id: "modal_link#{asset.id}",
-                data: { no_turbolink: true,
-                        id: true,
-                        'preview-url': asset_file_preview_path(asset),
-                        'order-atoz': order_atoz,
-                        'order-ztoa': order_ztoa,
-                        'order-new': i,
-                        'order-old': assets_count - i,
-                } do %>
-            <%= render partial: 'shared/asset_placeholder.html.erb', locals: { edit_page: false, asset: asset } %>
-        <% end %>
-    </div>
-<% end %>

app/views/steps/attachments/_list.html.erb

@@ -1,4 +1,5 @@
-<% assets = ordered_assets(step) %>
+<% assets = step.assets %>
+<% current_order = step.current_view_state(current_user).state.dig('assets', 'sort') %>
 <div class="col-xs-12">
 <hr>
 </div>
@@ -15,23 +16,21 @@
       <% if !(preview) && (can_manage_protocol_in_module?(@protocol) ||
                            can_manage_protocol_in_repository?(@protocol)) %>
         <%= render partial: '/assets/marvinjs/create_marvin_sketch_button.html.erb',
-                   locals: {  element_id: step.id, element_type: 'Step',  sketch_container: ".attachments#att-#{step.id}" } %>
+                   locals: {  element_id: step.id, element_type: 'Step',  sketch_container: ".attachments[data-step-id=#{step.id}]" } %>
         <%= render partial: '/assets/wopi/create_wopi_file_button.html.erb',
                    locals: {  element_id: step.id, element_type: 'Step' } %>
       <% end %>
-      <div class="dropdown attachments-order" id="dd-att-step-<%= step.id %>">
-        <button class="btn btn-default dropdown-toggle" type="button" id="sortMenu" data-toggle="dropdown" aria-haspopup="true" aria-expanded="true">
-          <span id="dd-att-step-<%= step.id %>-label">
-            <%= t("general.sort.#{step.current_view_state(current_user).state.dig('assets', 'sort')}_html") %>
+      <div class="dropdown attachments-order" data-step-id="<%= step.id %>" data-state-save-path="<%= update_view_state_step_path(step.id) %>">
+        <button class="btn btn-light dropdown-toggle" type="button" id="sortMenu" data-toggle="dropdown" aria-haspopup="true" aria-expanded="true">
+          <span class="selected-order">
+            <%= t("general.sort.#{current_order}_html") %>
           </span>
           <span class="caret"></span>
         </button>
-        <ul class="dropdown-menu" aria-labelledby="sortMenu" data-state-save-path="<%= update_view_state_step_path(step.id) %>">
+        <ul class="dropdown-menu" aria-labelledby="sortMenu" >
           <% ['new', 'old', 'atoz', 'ztoa'].each do |sort| %>
             <li>
-              <a data-order="<%= sort %>" onClick="reorderAttachments(this, '<%= step.id %>', '<%= sort %>')">
-                <%= t("general.sort.#{sort}_html") %>
-              </a>
+              <a data-order="<%= sort %>" class="change-order"><%= t("general.sort.#{sort}_html") %></a>
             </li>
           <% end %>
         </ul>
@@ -40,12 +39,9 @@
   </div>
 </div>
 
-<div class="col-xs-12 attachments" id="att-<%= step.id %>">
+<div class="col-xs-12 attachments" data-step-id="<%= step.id %>" data-order="<%= current_order %>">
   <% assets.each_with_index do |asset, i| %>
-    <% order_atoz = az_ordered_assets_index(step, asset.id) %>
-    <% order_ztoa = assets.length - az_ordered_assets_index(step, asset.id) %>
-    <%= render partial: 'steps/attachments/item.html.erb',
-      locals: { asset: asset, i: i, assets_count: assets.length, step: step, order_atoz: order_atoz, order_ztoa: order_ztoa } %>
+    <%= render partial: 'assets/asset.html.erb', locals: { asset: asset } %>
   <% end %>
 </div>
 <hr>