2497: Ability to edit skills

CHANGELOG.md

@@ -1,4 +1,5 @@
 ## Unreleased
+- Added ability to edit skill and grade descriptions
 - Added ability to generate student-level reports
 - Defaulted starting date for analytics to the last lesson's month
 - Replaced existing charts using the `Chart.js` library 

app/components/skill_form_component.html.erb

@@ -9,6 +9,7 @@
           <div class="col-span-6 lg:col-span-4">
             <%= f.label :skill_name, class: 'mt-1 block w-full rounded-md border-purple-500 shadow-xs focus:border-green-600 focus:ring-green-600 sm:text-sm' %>
             <%= f.text_field :skill_name, autofocus: true, class: 'mt-1 block w-full rounded-md border-purple-500 shadow-xs focus:border-green-600 focus:ring-green-600 sm:text-sm' %>
+            <%= render ValidationErrorComponent.new(model: @skill, key: :skill_name) %>
           </div>
           <div class="col-span-6 lg:col-span-4">
             <%= f.label :skill_description, class: 'mt-1 block w-full rounded-md border-purple-500 shadow-xs focus:border-green-600 focus:ring-green-600 sm:text-sm' %>
@@ -17,7 +18,7 @@
           <div class="col-span-2"></div>
           <div class="col-span-6 lg:col-span-2">
             <%= f.label :organization_id, class: 'mt-1 block w-full rounded-md border-purple-500 shadow-xs focus:border-green-600 focus:ring-green-600 sm:text-sm' %>
-            <%= f.collection_select :organization_id, @permitted_organizations, :id, :organization_name, {}, class: 'mt-1 block w-full rounded-md border-purple-500 shadow-xs focus:border-green-600 focus:ring-green-600 sm:text-sm' %>
+            <%= f.collection_select :organization_id, @permitted_organizations, :id, :organization_name, {}, disabled: @action == :update, class: 'mt-1 block w-full rounded-md border-purple-500 shadow-xs focus:border-green-600 focus:ring-green-600 sm:text-sm disabled:bg-gray-300 disabled:border-gray-300' %>
           </div>
         </div>
       </div>
@@ -26,28 +27,32 @@
       </div>
       <div class="mt-5 md:col-span-3 md:mt-0">
         <%= turbo_frame_tag f.field_id(:grades) do %>
-        <div id="grade_descriptors" class="grid grid-cols-6 gap-4" data-controller="association">
-          <%= f.fields_for :grade_descriptors do |gf| %>
-            <% id = SecureRandom.uuid %>
-            <div class="grid grid-cols-6 col-span-6 gap-4" data-association-target="removable" id="<%= id %>">
-            <div class="col-span-1">
-              <%= gf.label :mark, class: 'mt-1 block w-full rounded-md border-purple-500 shadow-xs focus:border-green-600 focus:ring-green-600 sm:text-sm' %>
-              <%= gf.number_field :mark, class: 'mt-1 block w-full rounded-md border-purple-500 shadow-xs focus:border-green-600 focus:ring-green-600 sm:text-sm' %>
-            </div>
-            <div class="col-span-3">
-              <%= gf.label :grade_description, class: 'mt-1 block w-full rounded-md border-purple-500 shadow-xs focus:border-green-600 focus:ring-green-600 sm:text-sm' %>
-              <%= gf.text_field :grade_description, rows: 5, class: 'mt-1 block w-full rounded-md border-purple-500 shadow-xs focus:border-green-600 focus:ring-green-600 sm:text-sm' %>
-            </div>
-            <div class="col-span-2 relative">
-              <button class="dangerous-button absolute bottom-0 cursor-pointer" data-action="association#remove" data-association-remove-id-param="<%= id %>"><%= t(:delete_grade) %></button>
-            </div>
-            </div>
-          <% end %>
-        </div>
-          <%= f.submit t(:add_grade), class: 'normal-button mt-4',
-                       name: :add_grade,
-                       data: {turbo_frame: f.field_id(:grades)}
-          %>
+          <div id="grade_descriptors" class="grid grid-cols-6 gap-4" data-controller="association">
+            <%= f.fields_for :grade_descriptors, f.object.grade_descriptors.sort_by(&:mark) do |gf| %>
+              <% id = SecureRandom.uuid %>
+              <div class="grid grid-cols-6 col-span-6 gap-4" data-association-target="removable" id="<%= id %>">
+              <div class="col-span-1">
+                <%= gf.label :mark, class: 'mt-1 block w-full rounded-md border-purple-500 shadow-xs focus:border-green-600 focus:ring-green-600 sm:text-sm' %>
+                <%= gf.number_field :mark, disabled: @action == :update, class: 'mt-1 block w-full rounded-md border-purple-500 shadow-xs focus:border-green-600 focus:ring-green-600 sm:text-sm disabled:bg-gray-300 disabled:border-gray-300' %>
+              </div>
+              <div class="col-span-3">
+                <%= gf.label :grade_description, class: 'mt-1 block w-full rounded-md border-purple-500 shadow-xs focus:border-green-600 focus:ring-green-600 sm:text-sm' %>
+                <%= gf.text_field :grade_description, rows: 5, class: 'mt-1 block w-full rounded-md border-purple-500 shadow-xs focus:border-green-600 focus:ring-green-600 sm:text-sm' %>
+              </div>
+              <div class="col-span-2 relative">
+                <% if @action != :update %>
+                  <button class="dangerous-button absolute bottom-0 cursor-pointer" data-action="association#remove" data-association-remove-id-param="<%= id %>"><%= t(:delete_grade) %></button>
+                <% end %>
+              </div>
+              </div>
+            <% end %>
+          </div>
+            <% if @action != :update %>
+              <%= f.submit t(:add_grade), class: 'normal-button mt-4',
+                         name: :add_grade,
+                         data: {turbo_frame: f.field_id(:grades)}
+              %>
+            <% end %>
         <% end %>
       </div>
     </div>

app/controllers/api/grade_descriptors_controller.rb

@@ -5,7 +5,7 @@ class GradeDescriptorsController < ApiController
     has_scope :exclude_deleted, type: :boolean
 
     def index
-      @grade_descriptors = apply_scopes(GradeDescriptor).all
+      @grade_descriptors = apply_scopes(GradeDescriptor.order(:mark)).all
       respond_with @grade_descriptors, include: included_params, meta: { timestamp: Time.zone.now }
     end
 

app/controllers/skills_controller.rb

@@ -27,6 +27,11 @@ def new
     authorize @skill
   end
 
+  def edit
+    @skill = Skill.find params.require(:id)
+    authorize @skill
+  end
+
   def create
     @skill = Skill.new skill_parameters
     authorize @skill
@@ -42,6 +47,19 @@ def create
     end
   end
 
+  def update
+    @skill = Skill.find params.require(:id)
+    @skill.assign_attributes skill_parameters
+    authorize @skill
+    if @skill.save
+      success(title: t(:skill_updated), text: t(:skill_updated_text, skill_name: @skill.skill_name))
+      redirect_to @skill
+    else
+      failure(title: t(:skill_invalid), text: t(:fix_form_errors))
+      render :edit, status: :unprocessable_content
+    end
+  end
+
   def destroy
     @skill = Skill.find params.require(:id)
     authorize @skill
@@ -89,7 +107,7 @@ def skill_subjects
   end
 
   def skill_parameters
-    params.require(:skill).permit(:skill_name, :organization_id, :skill_description, grade_descriptors_attributes: %i[mark grade_description _destroy])
+    params.require(:skill).permit(:skill_name, :organization_id, :skill_description, grade_descriptors_attributes: %i[id mark grade_description _destroy])
   end
 
   def render_deletion_error

app/models/skill.rb

@@ -28,7 +28,7 @@ class Skill < ApplicationRecord
   validate :grade_descriptors_must_have_unique_marks
 
   belongs_to :organization
-  has_many :grade_descriptors, dependent: :destroy, inverse_of: :skill
+  has_many :grade_descriptors, -> { order(:mark) }, dependent: :destroy, inverse_of: :skill
   has_many :assignments, dependent: :destroy
   has_many :subjects, through: :assignments, dependent: :restrict_with_error, inverse_of: :skills
 

app/views/skills/edit.html.erb

@@ -0,0 +1,14 @@
+<%- content_for :title, "#{t(:edit_skill)} - #{@skill.skill_name}" %>
+
+<%= render LayoutComponents::HeaderComponent.new(current_user: current_user, tabs: [
+  {title: CommonComponents::TagLabel.new(label: "#{t(:edit_skill)} - #{@skill.skill_name}", img_src: 'skill.svg'), href: '' },
+], buttons: [
+].compact)  do |h|
+  h.with_left do
+    render CommonComponents::Breadcrumbs.new(crumbs: [
+      { label: CommonComponents::TagLabel.new(label: t(:skills).capitalize, img_src: 'skill.svg'), href: skills_path }
+    ])
+  end
+end %>
+
+<%= render SkillFormComponent.new(skill: @skill, action: :update, current_user: current_user) %>

app/views/skills/show.html.erb

@@ -4,7 +4,8 @@
   {title: CommonComponents::TagLabel.new(label: @skill.skill_name, img_src: 'skill.svg'), href: '' },
 ], buttons: [
   (CommonComponents::FormButtonComponent.new(label: t(:restore_skill), path: undelete_skill_path(@skill), method: :post) if policy(@skill).destroy? && @skill.deleted_at),
-  (CommonComponents::DangerousFormButtonComponent.new(label: t(:delete_skill), path: skill_path(@skill), method: :delete) if policy(@skill).destroy? && !@skill.deleted_at)
+  (CommonComponents::DangerousFormButtonComponent.new(label: t(:delete_skill), path: skill_path(@skill), method: :delete) if policy(@skill).destroy? && !@skill.deleted_at),
+  (CommonComponents::ButtonComponent.new(label: t(:edit_skill), href: edit_skill_path(@skill)) if policy(@skill).edit?)
 ].compact)  do |h|
   h.with_left do
     render CommonComponents::Breadcrumbs.new(crumbs: [

config/locales/en.yml

@@ -297,6 +297,7 @@ en:
   skill: Skill
   skills: Skills
   new_skill: New Skill
+  edit_skill: Edit Skill
   skill_name: Skill Name
   add_skill: Add Skill
   skill_information: Skill Information
@@ -309,6 +310,8 @@ en:
   skill_deleted_text: Skill "%{skill_name}" deleted.
   skill_restored: Skill Restored
   skill_restored_text: Skill "%{skill_name}" restored.
+  skill_updated: Skill Updated
+  skill_updated_text: Skill "%{skill_name}" updated
   unable_to_delete_skill: Unable to delete skill
   unable_to_remove_skill_from_subject: Unable to remove skill "%{skill_name}" from subject
   unable_to_update_subject: Unable to update subject

config/routes.rb

@@ -87,7 +87,7 @@
     resources :students, controller: :student_lessons, only: %i[show update]
   end
   resources :subjects, only: %i[index new create show edit update]
-  resources :skills, only: %i[index create show new destroy] do
+  resources :skills, only: %i[index create show new destroy edit update] do
     member { post :undelete }
   end
 

spec/controllers/api/grade_descriptors_controller_spec.rb

@@ -15,6 +15,12 @@
     end
 
     it { should respond_with 200 }
+
+    it 'should return grade descriptors ordered by their marks' do
+      marks = response.parsed_body[:grade_descriptors].pluck(:mark)
+
+      expect(marks).to eq(marks.sort)
+    end
   end
 
   describe '#show' do

spec/controllers/skills_controller_spec.rb

@@ -125,6 +125,45 @@
       end
     end
 
+    describe '#edit' do
+      let(:skill) { create :skill }
+      before :each do
+        get :edit, params: { id: skill.id }
+      end
+
+      it { should respond_with :ok }
+      it { should render_template :edit }
+    end
+
+    describe '#update' do
+      before :each do
+        @skill = create :skill
+        @first_grade_descriptor = create :grade_descriptor, skill: @skill
+        @second_grade_descriptor = create :grade_descriptor, skill: @skill
+      end
+
+      it 'updates the skill\'s name' do
+        post :update, params: { id: @skill.id, skill: { skill_name: 'updated skill name' } }
+
+        expect(@skill.reload.skill_name).to eq 'updated skill name'
+      end
+
+      it 'updates the skill\'s description' do
+        post :update, params: { id: @skill.id, skill: { skill_description: 'updated skill description' } }
+
+        expect(@skill.reload.skill_description).to eq 'updated skill description'
+      end
+
+      it 'updates the skill\'s descriptors' do
+        post :update, params: { id: @skill.id, skill: { grade_descriptors_attributes:
+                                                              { '0' => { id: @first_grade_descriptor.id, grade_description: 'updated grade mark one' },
+                                                                '1' => { id: @second_grade_descriptor.id, grade_description: '' } } } }
+
+        @skill.reload
+        expect(@skill.grade_descriptors.map(&:grade_description)).to include 'updated grade mark one', ''
+      end
+    end
+
     describe '#destroy' do
       context 'Skill that has no grade and does not belong to a subject' do
         before :each do

spec/features/skill_features_spec.rb

@@ -111,5 +111,34 @@
         expect(skill.reload.deleted_at).to be_nil
       end
     end
+
+    describe 'Skill Editing' do
+      before :each do
+        @skill = create :skill, skill_name: 'To be Edited'
+        @grade_descriptor = create :grade_descriptor, skill: @skill
+        visit '/skills'
+      end
+
+      it 'edits an existing skill' do
+        find('div.table-cell', text: 'To be Edited', match: :first).click
+
+        click_link 'Edit Skill'
+
+        expect(page).to have_content "Edit Skill - #{@skill.skill_name}"
+
+        fill_in 'skill_skill_name', with: 'Updated Skill'
+        fill_in 'skill_skill_description', with: 'Updated description'
+        fill_in 'skill_grade_descriptors_attributes_0_grade_description', with: 'Updated grade description'
+
+        click_button 'Update Skill'
+        expect(page).to have_content 'Skill "Updated Skill" updated'
+
+        @skill.reload
+        expect(@skill.skill_name).to eq 'Updated Skill'
+        expect(@skill.skill_description).to eq 'Updated description'
+        expect(@skill.grade_descriptors.count).to eq 1
+        expect(@skill.grade_descriptors.first.grade_description).to eq 'Updated grade description'
+      end
+    end
   end
 end