Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow creating saved annotations by searching annotations #4965

Merged
merged 21 commits into from
Sep 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import { userAnnotationState } from "state/UserAnnotations";
import { savedAnnotationState } from "state/SavedAnnotations";
import { courseState } from "state/Courses";
import { exerciseState } from "state/Exercises";
import { userState } from "state/Users";

// Min and max of the annotation text is defined in the annotation model.
const maxLength = 10_000;
Expand Down Expand Up @@ -230,14 +229,13 @@ export class AnnotationForm extends watchMixin(ShadowlessLitElement) {
get potentialSavedAnnotationsExist(): boolean {
return (savedAnnotationState.getList(new Map([
["course_id", courseState.id.toString()],
["exercise_id", exerciseState.id.toString()],
["user_id", userState.id.toString()]
["exercise_id", exerciseState.id.toString()]
])) || []).length > 0;
}

get isTitleTaken(): boolean {
return savedAnnotationState.isTitleTaken(
this.savedAnnotationTitle, exerciseState.id, courseState.id, userState.id);
this.savedAnnotationTitle, exerciseState.id, courseState.id);
}

render(): TemplateResult {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { initTooltips } from "utilities";
import "components/saved_annotations/saved_annotation_icon";
import { annotationState } from "state/Annotations";
import { savedAnnotationState } from "state/SavedAnnotations";
import { isBetaCourse } from "saved_annotation_beta";

/**
* This component represents a single user annotation.
Expand Down Expand Up @@ -152,13 +153,15 @@ export class UserAnnotationComponent extends i18nMixin(ShadowlessLitElement) {
</li>
`);
}
if (this.data.permission.save) {
if (this.data.permission.save && isBetaCourse() && !this.data.saved_annotation_id) {
options.push(html`
<d-new-saved-annotation
from-annotation-id="${this.data.id}"
annotation-text="${this.data.annotation_text}"
.savedAnnotationId="${this.data.saved_annotation_id}">
</d-new-saved-annotation>
<li>
<d-new-saved-annotation
class="dropdown-item"
from-annotation-id="${this.data.id}"
annotation-text="${this.data.annotation_text}">
</d-new-saved-annotation>
</li>
`);
}
if (this.data.permission.destroy) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@
import { SavedAnnotation, savedAnnotationState } from "state/SavedAnnotations";
import "./saved_annotation_form";
import { modalMixin } from "components/modal_mixin";
import { isBetaCourse } from "saved_annotation_beta";
import { exerciseState } from "state/Exercises";
import { courseState } from "state/Courses";
import { userState } from "state/Users";
import { i18nMixin } from "components/meta/i18n_mixin";

/**
* This component represents an creation button for a saved annotation
Expand All @@ -17,42 +16,38 @@
*
* @prop {Number} fromAnnotationId - the id of the annotation which will be saved
* @prop {String} annotationText - the original text of the annotation which wil be saved
* @prop {Number} savedAnnotationId - the id of the saved annotation
* @prop {Number} exerciseId - the id of the exercise to which the annotation belongs
* @prop {Number} courseId - the id of the course to which the annotation belongs
*/
@customElement("d-new-saved-annotation")
export class NewSavedAnnotation extends modalMixin(ShadowlessLitElement) {
export class NewSavedAnnotation extends i18nMixin(modalMixin(ShadowlessLitElement)) {
@property({ type: Number, attribute: "from-annotation-id" })
fromAnnotationId: number;
@property({ type: String, attribute: "annotation-text" })
annotationText: string;
@property({ type: Number, attribute: "saved-annotation-id" })
savedAnnotationId: number;
@property({ type: Number, attribute: "exercise-id" })
exerciseId: number = exerciseState.id;

Check warning on line 29 in app/assets/javascripts/components/saved_annotations/new_saved_annotation.ts

View check run for this annotation

Codecov / codecov/patch

app/assets/javascripts/components/saved_annotations/new_saved_annotation.ts#L29

Added line #L29 was not covered by tests
@property({ type: Number, attribute: "course-id" })
courseId: number = courseState.id;

Check warning on line 31 in app/assets/javascripts/components/saved_annotations/new_saved_annotation.ts

View check run for this annotation

Codecov / codecov/patch

app/assets/javascripts/components/saved_annotations/new_saved_annotation.ts#L31

Added line #L31 was not covered by tests

@property({ state: true })
errors: string[];

savedAnnotation: SavedAnnotation;

get isAlreadyLinked(): boolean {
return this.savedAnnotationId != undefined;
}

get linkedSavedAnnotation(): SavedAnnotation {
return savedAnnotationState.get(this.savedAnnotationId);
}

get newSavedAnnotation(): SavedAnnotation {
return {
id: undefined,
// Take the first five words, with a max of 40 chars as default title
title: this.annotationText.split(/\s+/).slice(0, 5).join(" ").slice(0, 40),
annotation_text: this.annotationText
annotation_text: this.annotationText,
};
}

get isTitleTaken(): boolean {
const annotation = this.savedAnnotation || this.newSavedAnnotation;
return savedAnnotationState.isTitleTaken(
this.newSavedAnnotation.title, exerciseState.id, courseState.id, userState.id);
annotation.title, this.exerciseId, this.courseId);
}

async createSavedAnnotation(): Promise<void> {
Expand All @@ -67,6 +62,8 @@
});
this.errors = undefined;
this.hideModal();
const event = new CustomEvent("annotation-saved", { bubbles: true, composed: true });
this.dispatchEvent(event);

Check warning on line 66 in app/assets/javascripts/components/saved_annotations/new_saved_annotation.ts

View check run for this annotation

Codecov / codecov/patch

app/assets/javascripts/components/saved_annotations/new_saved_annotation.ts#L65-L66

Added lines #L65 - L66 were not covered by tests
} catch (errors) {
this.errors = errors;
}
Expand All @@ -88,6 +85,8 @@
<d-saved-annotation-form
.savedAnnotation=${this.newSavedAnnotation}
@change=${e => this.savedAnnotation = e.detail}
.courseId=${this.courseId}
.exerciseId=${this.exerciseId}
></d-saved-annotation-form>
`, html`
<button class="btn btn-text" @click=${() => this.createSavedAnnotation()}>
Expand All @@ -97,13 +96,21 @@
}

render(): TemplateResult {
return isBetaCourse() && !(this.isAlreadyLinked && this.linkedSavedAnnotation) ? html`
<li>
<a class="dropdown-item" @click="${() => this.showModal()}">
<i class="mdi mdi-content-save mdi-18"></i>
${I18n.t("js.saved_annotation.new.button_title")}
</a>
</li>
` : html``;
return html`
<a @click="${() => this.showModal()}">

Check warning on line 100 in app/assets/javascripts/components/saved_annotations/new_saved_annotation.ts

View check run for this annotation

Codecov / codecov/patch

app/assets/javascripts/components/saved_annotations/new_saved_annotation.ts#L99-L100

Added lines #L99 - L100 were not covered by tests
<i class="mdi mdi-content-save mdi-18"></i>
${I18n.t("js.saved_annotation.new.button_title")}
</a>
`;
}
}

export function initNewSavedAnnotationButtons(path: string): void {
const newSavedAnnotationElements = document.querySelectorAll("d-new-saved-annotation");
newSavedAnnotationElements.forEach((element: NewSavedAnnotation) => {
element.addEventListener("annotation-saved", () => {

Check warning on line 111 in app/assets/javascripts/components/saved_annotations/new_saved_annotation.ts

View check run for this annotation

Codecov / codecov/patch

app/assets/javascripts/components/saved_annotations/new_saved_annotation.ts#L109-L111

Added lines #L109 - L111 were not covered by tests
// redirect to the saved annotation list
window.location.href = path;

Check warning on line 113 in app/assets/javascripts/components/saved_annotations/new_saved_annotation.ts

View check run for this annotation

Codecov / codecov/patch

app/assets/javascripts/components/saved_annotations/new_saved_annotation.ts#L113

Added line #L113 was not covered by tests
});
});
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { searchQueryState } from "state/SearchQuery";
import { updateURLParameter } from "utilities";
import { html, TemplateResult } from "lit";
import { customElement } from "lit/decorators.js";
import { ShadowlessLitElement } from "components/meta/shadowless_lit_element";

Check warning on line 5 in app/assets/javascripts/components/saved_annotations/new_saved_annotation_link.ts

View check run for this annotation

Codecov / codecov/patch

app/assets/javascripts/components/saved_annotations/new_saved_annotation_link.ts#L1-L5

Added lines #L1 - L5 were not covered by tests

/**
* This component represents a link to the new saved annotations page
* It is responsible for adding the course_id and exercise_id query parameters to the link
*/
@customElement("d-new-saved-annotation-link")
export class NewSavedAnnotationLink extends ShadowlessLitElement {
get courseId(): string {
return searchQueryState.queryParams.get("course_id");

Check warning on line 14 in app/assets/javascripts/components/saved_annotations/new_saved_annotation_link.ts

View check run for this annotation

Codecov / codecov/patch

app/assets/javascripts/components/saved_annotations/new_saved_annotation_link.ts#L12-L14

Added lines #L12 - L14 were not covered by tests
}

get exerciseId(): string {
return searchQueryState.queryParams.get("exercise_id");

Check warning on line 18 in app/assets/javascripts/components/saved_annotations/new_saved_annotation_link.ts

View check run for this annotation

Codecov / codecov/patch

app/assets/javascripts/components/saved_annotations/new_saved_annotation_link.ts#L17-L18

Added lines #L17 - L18 were not covered by tests
}

get path(): string {
let path = "/saved_annotations/new";

Check warning on line 22 in app/assets/javascripts/components/saved_annotations/new_saved_annotation_link.ts

View check run for this annotation

Codecov / codecov/patch

app/assets/javascripts/components/saved_annotations/new_saved_annotation_link.ts#L21-L22

Added lines #L21 - L22 were not covered by tests
if (this.courseId) {
path = updateURLParameter(path, "course_id", this.courseId);

Check warning on line 24 in app/assets/javascripts/components/saved_annotations/new_saved_annotation_link.ts

View check run for this annotation

Codecov / codecov/patch

app/assets/javascripts/components/saved_annotations/new_saved_annotation_link.ts#L24

Added line #L24 was not covered by tests
}
if (this.exerciseId) {
path = updateURLParameter(path, "exercise_id", this.exerciseId);

Check warning on line 27 in app/assets/javascripts/components/saved_annotations/new_saved_annotation_link.ts

View check run for this annotation

Codecov / codecov/patch

app/assets/javascripts/components/saved_annotations/new_saved_annotation_link.ts#L27

Added line #L27 was not covered by tests
}
return path;

Check warning on line 29 in app/assets/javascripts/components/saved_annotations/new_saved_annotation_link.ts

View check run for this annotation

Codecov / codecov/patch

app/assets/javascripts/components/saved_annotations/new_saved_annotation_link.ts#L29

Added line #L29 was not covered by tests
}

render(): TemplateResult {
return html`

Check warning on line 33 in app/assets/javascripts/components/saved_annotations/new_saved_annotation_link.ts

View check run for this annotation

Codecov / codecov/patch

app/assets/javascripts/components/saved_annotations/new_saved_annotation_link.ts#L32-L33

Added lines #L32 - L33 were not covered by tests
<a href=${this.path} class="btn btn-fab hidden-print">
<i class="mdi mdi-plus"></i>
</a>
`;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,19 @@
* @element d-saved-annotation-form
*
* @prop {SavedAnnotation} savedAnnotation - the saved annotation to be edited in this form
* @prop {Number} exerciseId - the id of the exercise to which the annotation belongs
* @prop {Number} courseId - the id of the course to which the annotation belongs
*
* @fires change - on user changes in the form, event.detail has the new state of the SavedAnnotation
*/
@customElement("d-saved-annotation-form")
export class SavedAnnotationForm extends ShadowlessLitElement {
@property({ type: Object })
savedAnnotation: SavedAnnotation;
@property({ type: Number, attribute: "exercise-id" })
exerciseId: number;
@property({ type: Number, attribute: "course-id" })
courseId: number;

savedAnnotationChanged(): void {
const event = new CustomEvent("change", {
Expand Down Expand Up @@ -49,7 +55,9 @@
</label>
<d-saved-annotation-title-input
.value=${this.savedAnnotation.title}
@change=${e => this.updateTitle(e)}>
@change=${e => this.updateTitle(e)}

Check warning on line 58 in app/assets/javascripts/components/saved_annotations/saved_annotation_form.ts

View check run for this annotation

Codecov / codecov/patch

app/assets/javascripts/components/saved_annotations/saved_annotation_form.ts#L58

Added line #L58 was not covered by tests
.courseId=${this.courseId}
.exerciseId=${this.exerciseId}>
</d-saved-annotation-title-input>
</div>
<div class="field form-group">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { ShadowlessLitElement } from "components/meta/shadowless_lit_element";
import "components/datalist_input";
import { SavedAnnotation, savedAnnotationState } from "state/SavedAnnotations";
import { unsafeHTML } from "lit/directives/unsafe-html.js";
import { userState } from "state/Users";
import { courseState } from "state/Courses";
import { exerciseState } from "state/Exercises";

Expand Down Expand Up @@ -44,7 +43,6 @@ export class SavedAnnotationInput extends ShadowlessLitElement {
const savedAnnotations = savedAnnotationState.getList(new Map([
["course_id", courseState.id.toString()],
["exercise_id", exerciseState.id.toString()],
["user_id", userState.id.toString()],
["filter", this.__label]
]));
if (savedAnnotations === undefined) {
Expand All @@ -63,6 +61,10 @@ export class SavedAnnotationInput extends ShadowlessLitElement {
return this.savedAnnotations.map(sa => ({ label: sa.title, value: sa.id.toString(), extra: sa.annotation_text }));
}

get savedAnnotationsPath(): string {
return `/saved_annotations?course_id=${courseState.id}&exercise_id=${exerciseState.id}`;
}

get icon(): string {
if (this.selectedAnnotation == undefined) {
return "";
Expand Down Expand Up @@ -105,7 +107,7 @@ export class SavedAnnotationInput extends ShadowlessLitElement {
` : ""}
</div>
<div class="help-block">
${unsafeHTML(I18n.t("js.saved_annotation.input.help_html"))}
${unsafeHTML(I18n.t("js.saved_annotation.input.help_html", { path: this.savedAnnotationsPath }))}
</div>
</div>
`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { ShadowlessLitElement } from "components/meta/shadowless_lit_element";
import { savedAnnotationState } from "state/SavedAnnotations";
import { exerciseState } from "state/Exercises";
import { courseState } from "state/Courses";
import { userState } from "state/Users";

@customElement("d-saved-annotation-title-input")
export class SavedAnnotationTitleInput extends ShadowlessLitElement {
Expand All @@ -16,15 +15,13 @@ export class SavedAnnotationTitleInput extends ShadowlessLitElement {
exerciseId: number = exerciseState.id;
@property({ type: Number, attribute: "course-id" })
courseId: number = courseState.id;
@property({ type: Number, attribute: "user-id" })
userId: number = userState.id;

@property({ state: true })
_value: string;

get isTitleTaken(): boolean {
return savedAnnotationState.isTitleTaken(
this._value ?? this.value, this.exerciseId, this.courseId, this.userId, this.savedAnnotationId);
this._value ?? this.value, this.exerciseId, this.courseId, this.savedAnnotationId);
}

render(): TemplateResult {
Expand Down
4 changes: 2 additions & 2 deletions app/assets/javascripts/i18n/translations.json
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@
},
"input": {
"edited": "The current comment differs from the saved comment",
"help_html": "Click <a href=\"/saved_annotations\" target=\"_blank\" >here</a> to manage all saved comments.",
"help_html": "Click <a href=\"%{path}\" target=\"_blank\" >here</a> to manage all saved comments.",
"placeholder": "Search saved comment",
"title": "Reuse a saved comment"
},
Expand Down Expand Up @@ -837,7 +837,7 @@
},
"input": {
"edited": "De huidige opmerking verschilt van de opgeslagen opmerking",
"help_html": "Klik <a href=\"/saved_annotations\" target=\"_blank\" >hier</a> om alle opgeslagen opmerkingen te beheren.",
"help_html": "Klik <a href=\"%{path}\" target=\"_blank\" >hier</a> om alle opgeslagen opmerkingen te beheren.",
"placeholder": "Zoek opgeslagen opmerking",
"title": "Hergebruik een opgeslagen opmerking"
},
Expand Down
3 changes: 1 addition & 2 deletions app/assets/javascripts/state/SavedAnnotations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,12 +108,11 @@ class SavedAnnotationState extends State {
}
}

isTitleTaken(title: string, exerciseId: number, courseId: number, userId: number, savedAnnotationId: number = undefined): boolean {
isTitleTaken(title: string, exerciseId: number, courseId: number, savedAnnotationId: number = undefined): boolean {
const params = new Map([
["filter", title],
["exercise_id", exerciseId.toString()],
["course_id", courseId.toString()],
["user_id", userId.toString()],
]);
const list = this.getList(params);
return list?.find(annotation => annotation.title === title && annotation.id != savedAnnotationId) !== undefined;
Expand Down
23 changes: 21 additions & 2 deletions app/controllers/saved_annotations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
has_scope :by_exercise, as: 'exercise_id'
has_scope :by_filter, as: 'filter'

order_by :annotations_count, :title, :annotation_text
order_by :annotations_count, :title, :annotation_text, :created_at

def index
authorize SavedAnnotation
Expand All @@ -27,12 +27,31 @@
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]))
@annotations = apply_scopes(@saved_annotation.annotations.order_by_created_at(:DESC))
.paginate(page: parse_pagination_param(params[:page]))
end
format.json
format.js do
@annotations = apply_scopes(@saved_annotation.annotations.order_by_created_at(:DESC))

Check warning on line 35 in app/controllers/saved_annotations_controller.rb

View check run for this annotation

Codecov / codecov/patch

app/controllers/saved_annotations_controller.rb#L35

Added line #L35 was not covered by tests
.paginate(page: parse_pagination_param(params[:page]))
end
end
end

def new
authorize SavedAnnotation
@annotations = AnnotationPolicy::Scope.new(current_user, Annotation.all).resolve
@annotations = @annotations.where(saved_annotation_id: nil).where(user_id: current_user.id)
@courses = Course.where(id: @annotations.joins(:submission).pluck('submissions.course_id').uniq)
@exercises = Activity.where(id: @annotations.joins(:submission).pluck('submissions.exercise_id').uniq)
@annotations = apply_scopes(@annotations.order_by_created_at(:DESC))

Check warning on line 47 in app/controllers/saved_annotations_controller.rb

View check run for this annotation

Codecov / codecov/patch

app/controllers/saved_annotations_controller.rb#L42-L47

Added lines #L42 - L47 were not covered by tests
.includes(:course).includes(:user).includes(:submission)
.paginate(page: parse_pagination_param(params[:page]), per_page: parse_pagination_param(params[:per_page]))

@title = I18n.t('saved_annotations.new.title')
@crumbs = [[I18n.t('saved_annotations.index.title'), saved_annotations_path], [I18n.t('saved_annotations.new.title'), '#']]

Check warning on line 52 in app/controllers/saved_annotations_controller.rb

View check run for this annotation

Codecov / codecov/patch

app/controllers/saved_annotations_controller.rb#L51-L52

Added lines #L51 - L52 were not covered by tests
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'), '#']]
Expand Down
8 changes: 8 additions & 0 deletions app/javascript/packs/saved_annotation.js
Original file line number Diff line number Diff line change
@@ -1 +1,9 @@
import "components/saved_annotations/saved_annotation_title_input";
import "components/saved_annotations/new_saved_annotation";
import "components/search/sort_button";
import "components/saved_annotations/new_saved_annotation_link";
import { initNewSavedAnnotationButtons } from "components/saved_annotations/new_saved_annotation";
import { search } from "search";

dodona.initNewSavedAnnotationButtons = initNewSavedAnnotationButtons;
dodona.initSortButtons = () => search.autoSearch = true;
4 changes: 4 additions & 0 deletions app/models/annotation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ class Annotation < ApplicationRecord
scope :by_course, ->(course_id) { where(submission: Submission.in_course(Course.find(course_id))) }
scope :by_username, ->(name) { where(user: User.by_filter(name)) }
scope :by_exercise_name, ->(name) { where(submission: Submission.by_exercise_name(name)) }
scope :by_exercise, ->(exercise_id) { where(submission: { exercise_id: exercise_id }) }

scope :order_by_annotation_text, ->(direction) { reorder(annotation_text: direction) }
scope :order_by_created_at, ->(direction) { reorder(created_at: direction) }

before_validation :set_last_updated_by, on: :create
before_validation :set_course_id, on: :create
Expand Down
Loading
Loading