Merge pull request #4885 from dodona-edu/enhance/saved-annotation-page

Create separate show and edit page for saved-annotations
dodona-edu · Aug 16, 2023 · 7dc49cb · 7dc49cb
2 parents 8f1cbc0 + 9f6cd69
commit 7dc49cb
Show file tree

Hide file tree

Showing 21 changed files with 155 additions and 122 deletions.
diff --git a/app/assets/javascripts/components/saved_annotations/edit_saved_annotation.ts b/app/assets/javascripts/components/saved_annotations/edit_saved_annotation.ts
diff --git a/app/assets/javascripts/components/saved_annotations/saved_annotation_icon.ts b/app/assets/javascripts/components/saved_annotations/saved_annotation_icon.ts
@@ -26,9 +26,11 @@ export class SavedAnnotationIcon extends ShadowlessLitElement {
 
     render(): TemplateResult {
         return isBetaCourse() && this.isAlreadyLinked && this.savedAnnotation!= undefined ? html`
-            <i class="mdi mdi-link-variant mdi-18 annotation-meta-icon"
-               title="${I18n.t("js.saved_annotation.new.linked", { title: this.savedAnnotation.title })}"
-            ></i>
+            <a href="${this.savedAnnotation.url}" target="_blank">
+                <i class="mdi mdi-comment-bookmark-outline mdi-18 annotation-meta-icon"
+                   title="${I18n.t("js.saved_annotation.new.linked", { title: this.savedAnnotation.title })}"
+                ></i>
+            </a>
         ` : html``;
     }
 }
diff --git a/app/assets/javascripts/components/saved_annotations/saved_annotation_list.ts b/app/assets/javascripts/components/saved_annotations/saved_annotation_list.ts
@@ -2,7 +2,6 @@ import { customElement, property } from "lit/decorators.js";
 import { html, TemplateResult } from "lit";
 import { ShadowlessLitElement } from "components/meta/shadowless_lit_element";
 import { Pagination, SavedAnnotation, savedAnnotationState } from "state/SavedAnnotations";
-import "./edit_saved_annotation";
 import "components/pagination";
 import { searchQueryState } from "state/SearchQuery";
 
@@ -67,14 +66,13 @@ export class SavedAnnotationList extends ShadowlessLitElement {
                             <th>${I18n.t("js.saved_annotation.user")}</th>
                             <th>${I18n.t("js.saved_annotation.course")}</th>
                             <th>${I18n.t("js.saved_annotation.exercise")}</th>
-                            <th></th>
                         </thead>
                     `}
                     <tbody>
                         ${this.savedAnnotations.map(sa => html`
                             <tr>
                                 <td class="ellipsis-overflow" style="${this.small ? "max-width: 150px" : ""}" title="${sa.title}">
-                                    ${sa.title}
+                                    <a href="${sa.url}">${sa.title}</a>
                                     <p class="small text-muted">${I18n.t("js.saved_annotation.list.annotations_count", { count: sa.annotations_count })}</p>
                                 </td>
                                 ${ this.small ? "" : html`
@@ -83,9 +81,6 @@ export class SavedAnnotationList extends ShadowlessLitElement {
                                     <td><a href="${sa.course.url}">${sa.course.name}</a></td>
                                     <td><a href="${sa.exercise.url}">${sa.exercise.name}</a></td>
                                 `}
-                                <td class="actions">
-                                    <d-edit-saved-annotation .savedAnnotation=${sa}></d-edit-saved-annotation>
-                                </td>
                             </tr>
                         `)}
                     </tbody>

diff --git a/app/assets/javascripts/state/SavedAnnotations.ts b/app/assets/javascripts/state/SavedAnnotations.ts
@@ -10,6 +10,7 @@ export type SavedAnnotation = {
     title: string,
     id: number,
     annotation_text: string,
+    url?: string,
     user?: { name: string, url: string },
     exercise?: { name: string, url: string },
     course?: { name: string, url: string }
@@ -64,29 +65,6 @@ class SavedAnnotationState extends State {
         return savedAnnotation.id;
     }
 
-    async update(id: number, data: { saved_annotation: SavedAnnotation }): Promise<void> {
-        const url = `${URL}/${id}`;
-        const response = await fetch(url, {
-            method: "put",
-            body: JSON.stringify(data),
-            headers: { "Content-type": "application/json" },
-        });
-        if (response.status === 422) {
-            const errors = await response.json();
-            throw errors;
-        }
-        const savedAnnotation: SavedAnnotation = await response.json();
-        this.invalidate(savedAnnotation.id, savedAnnotation);
-    }
-
-    async delete(id: number): Promise<void> {
-        const url = `${URL}/${id}`;
-        await fetch(url, {
-            method: "delete",
-        });
-        this.invalidate(id);
-    }
-
     getList(params?: Map<string, string>, arrayParams?: Map<string, string[]>): Array<SavedAnnotation> | undefined {
         const url = addParametersToUrl(`${URL}.json`, params, arrayParams);
         delayerByURL.get(url)(() => {

diff --git a/app/assets/stylesheets/bootstrap_css_variable_overrides.css.scss b/app/assets/stylesheets/bootstrap_css_variable_overrides.css.scss
@@ -78,6 +78,7 @@
   --bs-link-color-rgb: var(--d-primary-rgb);
   --bs-link-decoration: none;
   --bs-link-hover-color: var(--d-on-primary-container);
+  --bs-link-hover-color-rgb: var(--d-on-primary-container-rgb);
   --bs-code-color: var(--d-secondary);
   --bs-highlight-bg: var(--d-surface-variant);
 

diff --git a/app/assets/stylesheets/theme/m3-theme-dark.css.scss b/app/assets/stylesheets/theme/m3-theme-dark.css.scss
@@ -10,6 +10,7 @@
   --d-on-primary: #{$primary-20};
   --d-on-primary-container: #{$primary-90};
   --d-primary-rgb: #{color.red($primary-80)}, #{color.green($primary-80)}, #{color.blue($primary-80)};
+  --d-on-primary-container-rgb: #{color.red($primary-90)}, #{color.green($primary-90)}, #{color.blue($primary-90)};
   --d-banner: #{$primary-20};
   --d-on-banner: #{$primary-90};
 

diff --git a/app/assets/stylesheets/theme/m3-theme-light.css.scss b/app/assets/stylesheets/theme/m3-theme-light.css.scss
@@ -12,6 +12,7 @@
   --d-on-primary: #{$primary-100};
   --d-on-primary-container: #{$primary-10};
   --d-primary-rgb: #{color.red($primary-40)}, #{color.green($primary-40)}, #{color.blue($primary-40)};
+  --d-on-primary-container-rgb: #{color.red($primary-10)}, #{color.green($primary-10)}, #{color.blue($primary-10)};
   --d-banner: #{$primary-30};
   --d-on-banner: #{$primary-90};
 

diff --git a/app/controllers/saved_annotations_controller.rb b/app/controllers/saved_annotations_controller.rb
@@ -1,6 +1,6 @@
 class SavedAnnotationsController < ApplicationController
   set_pagination_headers :saved_annotations, only: [:index]
-  before_action :set_saved_annotation, only: %i[show update destroy]
+  before_action :set_saved_annotation, only: %i[show update destroy edit]
 
   has_scope :by_user, as: 'user_id'
   has_scope :by_course, as: 'course_id'
@@ -9,12 +9,28 @@ class SavedAnnotationsController < ApplicationController
 
   def index
     authorize SavedAnnotation
+    @title = I18n.t('saved_annotations.index.title')
+    @crumbs = [[I18n.t('saved_annotations.index.title'), saved_annotations_path]]
     @saved_annotations = apply_scopes(policy_scope(SavedAnnotation.all))
                          .includes(:course).includes(:user).includes(:exercise)
                          .paginate(page: parse_pagination_param(params[:page]), per_page: parse_pagination_param(params[:per_page]))
   end
 
-  def show; end
+  def show
+    respond_to do |format|
+      format.html do
+        @title = @saved_annotation.title
+        @crumbs = [[I18n.t('saved_annotations.index.title'), saved_annotations_path], [@saved_annotation.title, saved_annotation_path(@saved_annotation)]]
+        @submissions = @saved_annotation.submissions.paginate(page: parse_pagination_param(params[:page]))
+      end
+      format.json
+    end
+  end
+
+  def edit
+    @title = I18n.t('saved_annotations.edit.title')
+    @crumbs = [[I18n.t('saved_annotations.index.title'), saved_annotations_path], [@saved_annotation.title, saved_annotation_path(@saved_annotation)], [I18n.t('saved_annotations.edit.title'), '#']]
+  end
 
   def create
     annotation = Annotation.find(params[:from])
@@ -35,14 +51,22 @@ def update
     respond_to do |format|
       if @saved_annotation.update(permitted_attributes(SavedAnnotation))
         format.json { render :show, status: :ok, location: @saved_annotation }
+        format.html do
+          redirect_to saved_annotation_path(@saved_annotation)
+        end
       else
         format.json { render json: @saved_annotation.errors.full_messages, status: :unprocessable_entity }
+        format.html do
+          @crumbs = [[I18n.t('saved_annotations.index.title'), saved_annotations_path], [@saved_annotation.title, saved_annotation_path(@saved_annotation)], [I18n.t('saved_annotations.edit.title'), '#']]
+          render :edit
+        end
       end
     end
   end
 
   def destroy
     @saved_annotation.destroy
+    redirect_to saved_annotations_url
   end
 
   private

diff --git a/app/models/saved_annotation.rb b/app/models/saved_annotation.rb
@@ -21,6 +21,7 @@ class SavedAnnotation < ApplicationRecord
   belongs_to :course
 
   has_many :annotations, dependent: :nullify
+  has_many :submissions, through: :annotations
 
   scope :by_user, ->(user_id) { where user_id: user_id }
   scope :by_course, ->(course_id) { where course_id: course_id }

diff --git a/app/policies/saved_annotation_policy.rb b/app/policies/saved_annotation_policy.rb
@@ -45,6 +45,10 @@ def show?
     user_admin_of_beta_course? && record_in_beta_course? && record&.user_id == user.id
   end
 
+  def edit?
+    update?
+  end
+
   def update?
     # EDIT AFTER CLOSED BETA
     user_admin_of_beta_course? && record_in_beta_course? && record&.user_id == user.id

diff --git a/app/views/saved_annotations/edit.html.erb b/app/views/saved_annotations/edit.html.erb
@@ -0,0 +1,45 @@
+<div class="row">
+  <div class="col-md-10 offset-md-1 col-12">
+    <div class="card">
+      <div class="card-title card-title-colored">
+        <h2 class="card-title-text"><%= t ".title" %></h2>
+      </div>
+      <div class="card-supporting-text">
+        <div class="callout callout-warning mt-0"><%= t ".warning_no_annotations_changed" %></div>
+        <%= form_for(@saved_annotation, html: { class: 'form-horizontal' }) do |f| %>
+          <% if @saved_annotation.errors.any? %>
+            <div class="callout callout-danger" id="error_explanation">
+              <h4><%= t('errors.validation_errors', count: @saved_annotation.errors.count) %></h4>
+              <ul>
+                <% @saved_annotation.errors.full_messages.each do |message| %>
+                  <li><%= message %></li>
+                <% end %>
+              </ul>
+            </div>
+          <% end %>
+          <div class="field form-group row">
+            <%= f.label :title, class: "col-sm-3 col-form-label" %>
+            <div class="col-sm-6"><%= f.text_field :title, class: "form-control" %></div>
+          </div>
+          <div class="field form-group row">
+            <%= f.label :annotation_text, class: "col-sm-3 col-form-label" %>
+            <div class="col-sm-6"><%= f.text_area :annotation_text, class: "form-control" %></div>
+            <span class="help-block offset-sm-3 col-sm-6"><%= t ".markdown_html" %></span>
+          </div>
+        <% end %>
+      </div>
+      <div class="card-actions card-border">
+        <% if policy(@saved_annotation).destroy? %>
+          <%= link_to @saved_annotation, method: :delete, data: { confirm: t("general.are_you_sure") }, class: "btn btn-filled with-icon d-btn-danger" do %>
+            <i class="mdi mdi-delete"></i>
+            <%= t ".destroy" %>
+          <% end %>
+        <% end %>
+        <button type="submit" class="btn btn-filled with-icon" form="<%= "edit_saved_annotation_#{@saved_annotation.id}"%>">
+          <i class="mdi mdi-send"></i>
+          <%= t ".save" %>
+        </button>
+      </div>
+    </div>
+  </div>
+</div>
diff --git a/app/views/saved_annotations/index.json.jbuilder b/app/views/saved_annotations/index.json.jbuilder
@@ -1,5 +1,6 @@
 json.array! @saved_annotations do |saved_annotation|
   json.extract! saved_annotation, :id, :title, :annotation_text, :user_id, :exercise_id, :course_id, :created_at, :updated_at, :annotations_count
+  json.url saved_annotation_url(saved_annotation)
   json.user do
     json.name saved_annotation.user.full_name
     json.url user_url(saved_annotation.user)

diff --git a/app/views/saved_annotations/show.html.erb b/app/views/saved_annotations/show.html.erb
@@ -0,0 +1,29 @@
+<div class="row">
+  <div class="col-md-10 offset-md-1 col-12">
+    <div class="card">
+      <div class="card-title card-title-colored">
+        <h2 class="card-title-text"><%= @saved_annotation.title %></h2>
+        <% if policy(@saved_annotation).edit? %>
+          <div class="card-title-fab">
+            <%= render 'fab_link', url: edit_saved_annotation_path(@saved_annotation), icon: 'pencil' %>
+          </div>
+        <% end %>
+      </div>
+      <div class="card-supporting-text">
+        <p><strong><%= @saved_annotation.title %></strong></p>
+        <blockquote><%= markdown @saved_annotation.annotation_text %></blockquote>
+        <%= t ".usage_info_html",
+          exercise_path: course_activity_path(@saved_annotation.course ,@saved_annotation.exercise),
+          exercise_name: @saved_annotation.exercise.name,
+          course_path: course_path(@saved_annotation.course),
+          course_name: @saved_annotation.course.name
+         %><br/>
+        <%= t ".count_info_html", count: @saved_annotation.annotations_count %>
+      </div>
+      <div class="card-supporting-text card-border">
+        <h4><%= t ".linked_submissions" %></h4>
+        <%= render partial: 'submissions/submissions_table', locals: {submissions: @submissions, exercise: @saved_annotation.exercise, course: @saved_annotation.course, user: nil} %>
+      </div>
+    </div>
+  </div>
+</div>
diff --git a/app/views/saved_annotations/show.json.jbuilder b/app/views/saved_annotations/show.json.jbuilder
@@ -1 +1,2 @@
 json.extract! @saved_annotation, :id, :title, :annotation_text, :user_id, :exercise_id, :course_id, :created_at, :updated_at
+json.url saved_annotation_url(@saved_annotation)
diff --git a/config/locales/models/en.yml b/config/locales/models/en.yml
@@ -208,3 +208,10 @@ en:
         institution_id: Institution ID
         institution: Institution
         style: Bootstrap style
+      saved_annotation:
+        title: Title
+        annotation_text: Text
+        course: Course
+        exercise: Exercise
+        annotations_count: Number of usages
+
diff --git a/config/locales/models/nl.yml b/config/locales/models/nl.yml
@@ -209,3 +209,9 @@ nl:
         institution_id: ID van onderwijsinstelling
         institution: Onderwijsinstelling
         style: Bootstrap-stijl
+      saved_annotation:
+        title: Titel
+        annotation_text: Tekst
+        course: Cursus
+        exercise: Oefening
+        annotations_count: "Aantal keer gebruikt"
diff --git a/config/locales/views/saved_annotations/en.yml b/config/locales/views/saved_annotations/en.yml
@@ -2,3 +2,13 @@ en:
   saved_annotations:
     index:
       title: Saved comments
+    show:
+      linked_submissions: Linked submissions
+      usage_info_html: This comment can be reused on all submissions for the exercise <a href="%{exercise_path}">%{exercise_name}</a> in the course <a href="%{course_path}">%{course_name}</a>.
+      count_info_html: This comment is used %{count} times.
+    edit:
+      title: Edit saved comment
+      markdown_html: <a href="https://docs.dodona.be/en/references/exercise-description/#markdown" target="_blank">Markdown</a> is supported.
+      warning_no_annotations_changed: Existing comments will not be updated. These changes will only be applied to new comments.
+      destroy: Delete
+      save: Save