Feature/589 edit skillset (#613)

* implement redirect to modal after edit button has been clicked

* use yarn as manager

* try to add modal

* revert back to npm

* clean up

* clean up

* clean up

* use new crud controller

* Add create functionality

* start implementing show error

* implement showing error messages when submiting invalid data

* ingore unnecessary test for this feature

* correct rubocop offenses

* style modal

* fix bug of reopening modal when cancel button was clicked

* adjust styling of modal

* only include children in category dropdown and change label of options

* adjust styling of danger alerts

* sort category options by the title of their parent

* beginn with implementation of edit skillset

* edit table row with stimulus instead of raw javascript

* use different approach to make table row editable

* use turbo-frame to edit row instead of bootstrap property

* implement selection change when category parent was selected

* finish form to edit skills

* decrease default bootstrap padding of select

* give style to other dropdown too

* fix error of not defined option values

* insert error tag below edit of row if form is invalid

* begin with testing

* finish test implementation

* increase width of content

* fix tests after changing category of skill

* change bootstrap classes on divs

* try using retry mechanism to fix test on pipeline

* use sleep method to resolve timing problem

* revert changes on yarn lock file

* replace sleep with capybara expectation

* refactor edit functionality by reloading whole row after dropdown change

* use CRUD controller impelementation as approach instead of custom update action

* fix error of wrong rerender by using requestSubmit method instead of submit

* fix text by adjusting name of query parameter and rework styles

* adjust ids of selects

* delete unnecessary within command in test

* rework tests by using different way to select option and adjusting ids

* refactor code after pullrequest review

* revert changes of capybara configuration

* resolve rubocop offenses

* remove unnecessary method and fix error by assigning update variable again

* fix tests by including authorization step

* resolve merge errors

---------

Co-authored-by: Yanick Minder <[email protected]>

app/assets/stylesheets/bootstrap.scss

@@ -1,2 +1,8 @@
 @import 'bootstrap/scss/bootstrap';
 @import 'bootstrap-icons/font/bootstrap-icons';
+
+.portfolio-select, .radar-select {
+  width: 100% !important;
+  padding-right: 0 !important;
+  padding-left: 0 !important;
+}

app/assets/stylesheets/styles.scss

@@ -8,8 +8,13 @@ $skills-gray: #f5f5f5;
   max-width: 60%;
 }
 
-.content {
+.pointer {
+  cursor: pointer;
+}.modal-dialog {
   max-width: 60%;
+}
+.content {
+  max-width: 80%;
   flex-grow: 1;
 }
 
@@ -45,6 +50,11 @@ pzsh-topbar {
   color: $skills-text-gray;
 }
 
+.table-row:hover {
+  background-color: $light;
+}
+
+
 .person-profile-header {
   background: $skills-gray;
   height: 3.25rem;

app/controllers/crud_controller.rb

@@ -10,6 +10,7 @@
 # With the help of additional callbacks, it is possible to hook into the
 # action procedures without overriding the entire method.
 class CrudController < ListController
+  include ParamConverters
 
   class_attribute :permitted_attrs
 
@@ -64,7 +65,7 @@ def create(**options, &block)
       assign_attributes
       created = with_callbacks(:create, :save) { entry.save }
       respond(created,
-              **options.reverse_merge(status: :created, render_on_failure: :new),
+              **options.reverse_merge(status: :created, render_on_unsaved: :new),
               &block)
       raise ActiveRecord::Rollback unless created
     end
@@ -82,16 +83,26 @@ def create(**options, &block)
   # in the given block will take precedence over the one defined here.
   #
   # Specify a :location option if you wish to do a custom redirect.
+
+  # rubocop:disable Metrics/MethodLength
   def update(**options, &block)
     model_class.transaction do
-      assign_attributes
-      updated = with_callbacks(:update, :save) { entry.save }
-      respond(updated,
-              **options.merge(status: :ok, render_on_failure: :edit),
-              &block)
-      raise ActiveRecord::Rollback unless updated
+      if assign_attributes
+        updated = false
+        if true?(params[:validate_only])
+          entry.validate
+        else
+          updated = with_callbacks(:update, :save) { entry.save }
+        end
+
+        respond(updated,
+                **options.merge(status: :ok, render_on_unsaved: :edit),
+                &block)
+        raise ActiveRecord::Rollback unless updated
+      end
     end
   end
+  # rubocop:enable Metrics/MethodLength
 
   #   DELETE /entries/1
   #   DELETE /entries/1.json
@@ -160,7 +171,7 @@ def respond(success, **options)
       if success
         render_on_success(format, **options)
       else
-        render_on_error(format, **options)
+        render_on_unsaved(format, **options)
       end
     end
   end
@@ -170,17 +181,17 @@ def render_on_success(format, **options)
     format.json { render_success_json(options[:status]) }
   end
 
-  def render_on_error(format, **options)
-    format.turbo_stream { render options[:render_on_failure], status: options[:status] }
-    format.html { render_or_redirect_on_failure(**options) }
-    format.json { render_failure_json }
+  def render_on_unsaved(format, **options)
+    format.turbo_stream { render options[:render_on_unsaved], status: options[:status] }
+    format.html { render_or_redirect_on_unsaved(**options) }
+    format.json { render_unsaved_json }
   end
 
-  # If the option :render_on_failure is given, render the corresponding
+  # If the option :render_on_unsaved is given, render the corresponding
   # template, otherwise redirect.
-  def render_or_redirect_on_failure(**options)
-    if options[:render_on_failure]
-      render options[:render_on_failure], status: options[:status]
+  def render_or_redirect_on_unsaved(**options)
+    if options[:render_on_unsaved]
+      render options[:render_on_unsaved], status: options[:status]
     else
       redirect_on_failure(**options)
     end
@@ -213,7 +224,7 @@ def render_success_json(status)
   end
 
   # Render a json with the errors.
-  def render_failure_json
+  def render_unsaved_json
     render json: entry.errors, status: :unprocessable_entity
   end
 

app/controllers/skills_controller.rb

@@ -2,6 +2,7 @@
 
 class SkillsController < CrudController
   include ExportController
+  before_action :update_category_parent, only: [:update]
 
   before_action :render_unauthorized, except: %i[index show unrated_by_person]
 
@@ -45,4 +46,15 @@ def export
   def disposition
     content_disposition('attachment', filename('skillset', nil, 'csv'))
   end
+
+  # rubocop:disable Metrics/AbcSize
+  def update_category_parent
+    current_parent_id = Category.find(params[:skill][:category_id]).parent.id
+    new_parent_id = params[:skill].delete(:category_parent).to_i
+    if current_parent_id != new_parent_id
+      entry.category_id = Category.find(new_parent_id).children.first.id
+      params[:skill].delete(:category_id)
+    end
+  end
+  # rubocop:enable Metrics/AbcSize
 end

app/javascript/controllers/index.js

@@ -18,3 +18,7 @@ application.register("person-roles", PersonRolesController)
 
 import RemoteModalController from "./remote_modal_controller"
 application.register("remote-modal", RemoteModalController)
+
+import DropdownLinksController from "./dropdown_controller"
+application.register("dropdown", DropdownLinksController)
+

app/views/skills/_row.html.haml

@@ -0,0 +1,21 @@
+= form_with(model: @skill) do |f|
+  .row.border.border-top.table-light.table-hover.table-row
+    .col-2.d-flex.align-items-center
+      = f.text_field :title, class: "form-control", value: @skill.title
+    .col-1.bg-light.d-flex.align-items-center
+      =@skill.people.count
+    .col-3.d-flex.align-items-center
+      = f.collection_select :category_parent, Category.all_parents { |category| category.title}, :id, :title, {:selected => @skill.category.parent.id}, class: "form-select w-100", onchange: "this.form.action = this.form.action + '?validate_only=true'; this.form.requestSubmit()"
+    .col-2.bg-light.d-flex.align-items-center
+      = f.collection_select :category_id, entry.category.parent.children, :id, :title, {:selected => entry.category.id}, class: "form-select w-100"
+    .col-1.d-flex.align-items-center
+      =f.check_box :default_set, class: "form-check-input", checked: entry.default_set
+    .col-1.bg-light.d-flex.align-items-center
+      = f.select :radar, Settings.radar, {:selected => entry.radar}, class: "form-select radar-select"
+    .col-1.d-flex.align-items-center
+      = f.select :portfolio, Settings.portfolio, {:selected => entry.portfolio}, class: "form-select portfolio-select"
+    .col-1
+      %div.h-100.d-flex.justify-content-center.align-items-center
+        = image_submit_tag("/assets/floppy2-fill.svg")
+        = link_to skills_path, class: "ms-3" do
+          %img.pointer{:src=> "/assets/x.svg",:height=>"16"}

app/views/skills/edit.html.haml

@@ -0,0 +1,4 @@
+%div
+  %h1.font-bold.text-2xl.mb-3 Editing Skill
+  %turbo-frame{id: "#{dom_id @skill}"}
+    = render "skills/row", skill: @skill

app/views/skills/edit.turbo_stream.erb

@@ -0,0 +1,12 @@
+<%= turbo_stream.update "#{dom_id @skill}" do %>
+  <%= render "row", skill: @skill %>
+  <% if @skill.errors.any? %>
+    <div class="alert alert-danger mt-2">
+      <ul class="mb-0">
+        <% @skill.errors.full_messages.each do |error| %>
+          <li><%= error %></li>
+        <% end %>
+      </ul>
+    </div>
+  <% end %>
+<% end %>

app/views/skills/index.html.haml

@@ -1,24 +1,41 @@
 %div.d-flex.justify-content-end
   =link_to image_tag("plus-lg.svg", class: "text-primary")+ "Neuer Skill", new_skill_path, class: "btn text-primary", data: { turbo_frame: "remote_modal" }
 
-%table.table.table-hover
-  %thead.table-light
-    %tr
-      %th{scope: "col"}= t "skills.table.skill"
-      %th.table-secondary{scope: "col"}= t "skills.table.members"
-      %th{scope: "col"}= t "skills.table.category"
-      %th.table-secondary{scope: "col"}= t "skills.table.subcategory"
-      %th{scope: "col"}= t "skills.table.default_set"
-      %th.table-secondary{scope: "col"}= t "skills.table.radar"
-      %th{scope: "col"}= t "skills.table.modify"
-  %tbody
-    - @skills.each_with_index do |skill|
-      %tr
-        %td= skill.title
-        %td.table-light= skill.people.count
-        %td= skill.category.title
-        %td.table-light= skill.category.parent&.title
-        %td= skill.default_set.nil? ? "Neu" : (skill.default_set? ? "Ja" : "Nein")
-        %td.table-light= skill.radar
-        %td
-          %img{:src=> "/assets/pencil-square.svg",:height=>"16"}
+.row.bg-light-subtle.border.border-top.border-5.border-secondary.border-top-0.border-start-0.border-end-0
+  .col-2.d-flex.align-items-center
+    %strong= t "skills.table.skill"
+  .col-1.bg-secondary-subtle.d-flex.align-items-center
+    %strong= t "skills.table.members"
+  .col-3.d-flex.align-items-center
+    %strong= t "skills.table.category"
+  .col-2.bg-secondary-subtle.d-flex.align-items-center
+    %strong= t "skills.table.subcategory"
+  .col-1.d-flex.align-items-center
+    %strong= t "skills.table.default_set"
+  .col-1.bg-secondary-subtle.d-flex.align-items-center
+    %strong= t "skills.table.radar"
+  .col-1.d-flex.align-items-center
+    %strong= t "skills.table.portfolio"
+  .col-1.bg-secondary-subtle.d-flex.align-items-center
+    %strong= t "skills.table.modify"
+- @skills.each do |skill|
+  %turbo-frame{id: "#{dom_id skill}"}
+    .row.border.border-top.table-light.tableform-hover.table-row
+      .col-2.d-flex.align-items-center
+        =skill.title
+      .col-1.bg-light.d-flex.align-items-center
+        =skill.people.count
+      .col-3.d-flex.align-items-center
+        =skill.category.parent.title
+      .col-2.bg-light.d-flex.align-items-center
+        =skill.category.title
+      .col-1.d-flex.align-items-center
+        =skill.default_set.nil? ? "Neu" : (skill.default_set? ? "Ja" : "Nein")
+      .col-1.bg-light.d-flex.align-items-center
+        =skill.radar
+      .col-1.d-flex.align-items-center
+        =skill.portfolio
+      .col-1.bg-light
+        %div.h-100.d-flex.justify-content-center.align-items-center
+          =link_to edit_skill_path(skill), class: "btn bg-gray-100" do
+            %img.pointer.edit-button{:src=> "/assets/pencil-square.svg",:height=>"16"}

app/views/skills/show.html.haml

@@ -1,7 +1,20 @@
-%div
-  %h1
-    = @skill.title
-  %div
-    %h5
-= link_to "Skill bearbeiten",  edit_skill_path(@skill)
-= link_to "Alle Skills ansehen",  skills_path
+%turbo-frame{id: "#{dom_id @skill}"}
+  .row.border.border-top.table-light.tableform-hover.table-row
+    .col-2.d-flex.align-items-center
+      =@skill.title
+    .col-1.bg-light.d-flex.align-items-center
+      =@skill.people.count
+    .col-3.d-flex.align-items-center
+      =@skill.category.parent.title
+    .col-2.bg-light.d-flex.align-items-center
+      =@skill.category.title
+    .col-1.d-flex.align-items-center
+      =@skill.default_set.nil? ? "Neu" : (@skill.default_set? ? "Ja" : "Nein")
+    .col-1.bg-light.d-flex.align-items-center
+      =@skill.radar
+    .col-1.bg-light.d-flex.align-items-center
+      =@skill.portfolio
+    .col-1
+      %div.h-100.d-flex.justify-content-center.align-items-center
+        =link_to edit_skill_path(@skill), class: "btn bg-gray-100" do
+          %img.pointer.edit-button{:src=> "/assets/pencil-square.svg",:height=>"16"}

config/locales/de.yml

@@ -7,6 +7,7 @@ de:
       subcategory: Subkategorie
       default_set: Default-Set
       radar: Radar
+      portfolio: Portfolio
       modify: Ändern
   marital_statuses:
     single: ledig

config/locales/en.yml

@@ -7,6 +7,7 @@ en:
       subcategory: Subcategory
       default_set: Defaultset
       radar: Radar
+      portfolio: Portfolio
       modify: Modify
   marital_statuses:
     single: single