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

Make saved annotations available in all courses #4992

Merged
merged 3 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
@@ -1,7 +1,6 @@
import { customElement, property } from "lit/decorators.js";
import { html, TemplateResult } from "lit";
import { ShadowlessLitElement } from "components/meta/shadowless_lit_element";
import { isBetaCourse } from "saved_annotation_beta";
import { watchMixin } from "components/meta/watch_mixin";
import { createRef, Ref, ref } from "lit/directives/ref.js";
import "components/saved_annotations/saved_annotation_input";
Expand Down Expand Up @@ -223,7 +222,7 @@ export class AnnotationForm extends watchMixin(ShadowlessLitElement) {
}

get canSaveAnnotation(): boolean {
return !annotationState.isQuestionMode && /* REMOVE AFTER CLOSED BETA */ isBetaCourse();
return !annotationState.isQuestionMode;
}

get potentialSavedAnnotationsExist(): boolean {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ 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 @@ -153,7 +152,7 @@ export class UserAnnotationComponent extends i18nMixin(ShadowlessLitElement) {
</li>
`);
}
if (this.data.permission.save && isBetaCourse() && !this.data.saved_annotation_id) {
if (this.data.permission.save && !this.data.saved_annotation_id) {
options.push(html`
<li>
<d-new-saved-annotation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 { SavedAnnotation, savedAnnotationState } from "state/SavedAnnotations";
import { isBetaCourse } from "saved_annotation_beta";

/**
* Shows a link icon with some info on hover about the linked saved annotation
Expand All @@ -25,7 +24,7 @@ export class SavedAnnotationIcon extends ShadowlessLitElement {
}

render(): TemplateResult {
return isBetaCourse() && this.isAlreadyLinked && this.savedAnnotation!= undefined ? html`
return this.isAlreadyLinked && this.savedAnnotation!= undefined ? html`
<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 })}"
Expand Down
9 changes: 0 additions & 9 deletions app/assets/javascripts/saved_annotation_beta.ts

This file was deleted.

2 changes: 0 additions & 2 deletions app/assets/javascripts/state/UserAnnotations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,6 @@ interface UserAnnotationData {
user: UserAnnotationUserData;
type: AnnotationType;
last_updated_by: UserAnnotationUserData;
// REMOVE AFTER CLOSED BETA
course_id: number;
question_state?: QuestionState;
newer_submission_url?: string | null;
responses: UserAnnotationData[];
Expand Down
38 changes: 7 additions & 31 deletions app/policies/saved_annotation_policy.rb
Original file line number Diff line number Diff line change
@@ -1,48 +1,26 @@
class SavedAnnotationPolicy < ApplicationPolicy
# REMOVE AFTER CLOSED BETA
BETA_COURSES = [10, 773, 1151, 1659, 1662, 2258, 2263].freeze

class Scope < ApplicationPolicy::Scope
def resolve
if user&.zeus?
scope.all
elsif user
# EDIT AFTER CLOSED BETA
scope.where(user_id: user.id).where(course_id: BETA_COURSES)
elsif user&.a_course_admin?
scope.where(user_id: user.id)
else
scope.none
end
end
end

# REMOVE AFTER CLOSED BETA
def beta_course?(course_id)
course_id.in? BETA_COURSES
end

# REMOVE AFTER CLOSED BETA
def user_admin_of_beta_course?
user&.zeus? || user&.administrating_courses&.pluck(:id)&.any? { |c| c.in? BETA_COURSES }
end

# REMOVE AFTER CLOSED BETA
def record_in_beta_course?
beta_course?(record.course_id)
end

def index?
# EDIT AFTER CLOSED BETA
user&.a_course_admin? && user_admin_of_beta_course?
user&.a_course_admin?
end

def create?
# EDIT AFTER CLOSED BETA
record.course_id.present? && user&.course_admin?(record.course) && user_admin_of_beta_course? && record_in_beta_course? && record&.user_id == user.id
record.course_id.present? && user&.course_admin?(record.course) && record&.user_id == user.id
end

def show?
# EDIT AFTER CLOSED BETA
user_admin_of_beta_course? && record_in_beta_course? && record&.user_id == user.id
record&.user_id == user.id
end

def new?
Expand All @@ -54,13 +32,11 @@ def edit?
end

def update?
# EDIT AFTER CLOSED BETA
user_admin_of_beta_course? && record_in_beta_course? && record&.user_id == user.id
record&.user_id == user.id
end

def destroy?
# EDIT AFTER CLOSED BETA
user_admin_of_beta_course? && record_in_beta_course? && record&.user_id == user.id
record&.user_id == user.id
end

def permitted_attributes
Expand Down
2 changes: 1 addition & 1 deletion test/controllers/saved_annotation_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ class SavedAnnotationControllerTest < ActionDispatch::IntegrationTest
crud_helpers SavedAnnotation, attrs: %i[title annotation_text]

def setup
@course = create :course, id: 10 # COURSE ID CAN BE REMOVED AFTER CLOSED BETA
@course = create :course
@user = users(:staff)
CourseMembership.create(course: @course, user: @user, status: :course_admin)
@instance = create :saved_annotation, user: @user, course: @course
Expand Down
6 changes: 0 additions & 6 deletions test/javascript/code_listing.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,6 @@ test("annotations should be transmitted into view", async () => {
"created_at": "2023-03-02T15:15:48.776+01:00",
"url": "http://dodona.localhost:3000/nl/annotations/1.json",
"last_updated_by": { "name": "Zeus Kronosson" },
"course_id": 1,
"responses": [],
"type": "question",
"annotation_text": "This could be shorter",
Expand All @@ -236,7 +235,6 @@ test("annotations should be transmitted into view", async () => {
"created_at": "2023-03-02T15:15:48.776+01:00",
"url": "http://dodona.localhost:3000/nl/annotations/1.json",
"last_updated_by": { "name": "Zeus Kronosson" },
"course_id": 1,
"responses": [],
"type": "question",
"annotation_text": "This should be faster",
Expand Down Expand Up @@ -264,7 +262,6 @@ test("feedback table should support more than 1 annotation per row", async () =>
"created_at": "2023-03-02T15:15:48.776+01:00",
"url": "http://dodona.localhost:3000/nl/annotations/1.json",
"last_updated_by": { "name": "Zeus Kronosson" },
"course_id": 1,
"responses": [],
"type": "question",
"annotation_text": "This could be shorter",
Expand All @@ -287,7 +284,6 @@ test("feedback table should support more than 1 annotation per row", async () =>
"created_at": "2023-03-02T15:15:48.776+01:00",
"url": "http://dodona.localhost:3000/nl/annotations/1.json",
"last_updated_by": { "name": "Zeus Kronosson" },
"course_id": 1,
"responses": [],
"type": "question",
"annotation_text": "This should be faster",
Expand Down Expand Up @@ -315,7 +311,6 @@ test("feedback table should be able to contain both machine annotations and user
"created_at": "2023-03-02T15:15:48.776+01:00",
"url": "http://dodona.localhost:3000/nl/annotations/1.json",
"last_updated_by": { "name": "Zeus Kronosson" },
"course_id": 1,
"responses": [],
"type": "question",
"annotation_text": "This could be shorter",
Expand All @@ -338,7 +333,6 @@ test("feedback table should be able to contain both machine annotations and user
"created_at": "2023-03-02T15:15:48.776+01:00",
"url": "http://dodona.localhost:3000/nl/annotations/1.json",
"last_updated_by": { "name": "Zeus Kronosson" },
"course_id": 1,
"responses": [],
"type": "question",
"annotation_text": "This should be faster",
Expand Down