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

Add edit submission link when evaluating a submission #5190

Merged
merged 9 commits into from
Dec 19, 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
8 changes: 6 additions & 2 deletions app/assets/javascripts/code_listing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,18 @@ import {

const MARKING_CLASS = "marked";

function initAnnotations(submissionId: number, courseId: number, exerciseId: number, userId: number, code: string, questionMode = false): void {
function initAnnotations(submissionId: number, courseId: number, exerciseId: number, userId: number, code: string, userIsStudent: boolean, canSubmitAsOwn: boolean): void {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is becoming a bit of an anti-pattern. For such long list of options, an object might be better (but I don't know if we already do that in other places).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is only used in a single place, so making it an object would be very artificial, definitely with the rather specific options.

Although a case could be made to group the first options into a submission object. This would be more reusable in the frontend. But I see no reason to refactor that right now.

userAnnotationState.reset();
submissionState.code = code;
courseState.id = courseId;
exerciseState.id = exerciseId;
userState.id = userId;
submissionState.id = submissionId;
annotationState.isQuestionMode = questionMode;
annotationState.isQuestionMode = userIsStudent;

if (canSubmitAsOwn) {
userState.addPermission("submission.submit_as_own");
}

const table = document.querySelector<HTMLTableElement>("table.code-listing");
const rows = table.querySelectorAll("tr");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import "components/annotations/hidden_annotations_dot";
import { i18nMixin } from "components/meta/i18n_mixin";
import { userState } from "state/Users";
import { annotationState } from "state/Annotations";
import { submissionState } from "state/Submissions";


/**
Expand Down Expand Up @@ -38,6 +39,11 @@ export class AnnotationOptions extends i18nMixin(ShadowlessLitElement) {
${this.addAnnotationTitle}
</button>
` : html``}
${submissionState.canResubmitSubmission ? html`
<a class="btn btn-text resubmit-btn" href="${submissionState.resubmitPath}" target="_blank">
${I18n.t("js.feedbacks.submission.submit")}
</a>
` : html``}
<span class="flex-spacer"></span>
<d-annotations-toggles></d-annotations-toggles>
</div>
Expand Down
10 changes: 10 additions & 0 deletions app/assets/javascripts/i18n/translations.json
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,11 @@
"favorite-course-do": "Favorite",
"favorite-course-failed": "Favoriting course failed",
"favorite-course-succeeded": "Favorited course",
"feedbacks": {
"submission": {
"submit": "Submit this solution"
}
},
"institutions": "Institutions",
"judges": "Judges",
"label-undeletable": "This label can't be deleted because it was set in the dirconfig file of a parent directory of the learning activity.",
Expand Down Expand Up @@ -753,6 +758,11 @@
"favorite-course-do": "Voeg toe aan favorieten",
"favorite-course-failed": "Cursus aan favorieten toevoegen mislukt",
"favorite-course-succeeded": "Cursus toegevoegd aan favorieten",
"feedbacks": {
"submission": {
"submit": "Deze oplossing indienen"
}
},
"institutions": "Onderwijsinstellingen",
"judges": "Judges",
"label-undeletable": "Dit label kan niet verwijderd worden omdat het werd ingesteld in het dirconfig bestand van een bovenliggende map.",
Expand Down
11 changes: 11 additions & 0 deletions app/assets/javascripts/state/Submissions.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { State } from "state/state_system/State";
import { stateProperty } from "state/state_system/StateProperty";
import { userState } from "state/Users";
import { courseState } from "state/Courses";
import { exerciseState } from "state/Exercises";

class SubmissionState extends State {
@stateProperty id: number;
Expand All @@ -14,6 +17,14 @@ class SubmissionState extends State {
get code(): string {
return this._code;
}

get canResubmitSubmission(): boolean {
return userState.hasPermission("submission.submit_as_own");
}

get resubmitPath(): string {
return `/courses/${courseState.id}/exercises/${exerciseState.id}/?edit_submission=${submissionState.id}`;
}
}

export const submissionState = new SubmissionState();
2 changes: 1 addition & 1 deletion app/assets/javascripts/state/Users.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { stateProperty } from "state/state_system/StateProperty";
import { ThemeOption } from "state/Theme";
import { fetch } from "utilities";

export type Permission = "annotation.create"
export type Permission = "annotation.create" | "submission.submit_as_own";

class UserState extends State {
@stateProperty id: number;
Expand Down
5 changes: 5 additions & 0 deletions app/assets/stylesheets/components/card.css.scss
Original file line number Diff line number Diff line change
Expand Up @@ -381,3 +381,8 @@ a.card-title-link:hover {
flex-direction: column;
justify-content: space-evenly;
}

.card-title .btn-icon {
margin-top: -6px;
margin-bottom: -6px;
}
5 changes: 0 additions & 5 deletions app/assets/stylesheets/models/activities.css.scss
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,6 @@
line-height: normal;
margin-left: auto;
}

.btn-icon {
margin-top: -6px;
margin-bottom: -6px;
}
}

.card-supporting-text {
Expand Down
25 changes: 25 additions & 0 deletions app/assets/stylesheets/models/submissions.css.scss
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,23 @@
width: 100%;
display: flex;

.resubmit-btn {
margin-left: 0.5em;
}

@include media-breakpoint-down(lg) {
flex-direction: column;

.btn {
width: fit-content;
margin-bottom: 0.5em;
}

.resubmit-btn {
margin-left: 0;
}
}

&.sticky {
margin-bottom: 0;
margin-top: -16px;
Expand All @@ -339,6 +356,14 @@
span {
margin-right: 0.5em;
}

@include media-breakpoint-down(xl) {
margin-left: 0;

span {
display: none;
}
}
}

.diff-switch-buttons {
Expand Down
4 changes: 3 additions & 1 deletion app/helpers/renderers/feedback_code_renderer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,15 @@ def add_messages(submission, messages, user)
else
AnnotationPolicy.new(user, Annotation.new(submission: submission, user: user)).create?
end
user_is_owner = submission.user == user
can_submit_as_own = !user_is_owner && SubmissionPolicy.new(user, submission).create?

@builder.tag!('d-annotation-options') {}

@builder.script(type: 'application/javascript') do
@builder << <<~HEREDOC
window.dodona.ready.then(() => {
window.dodona.codeListing.initAnnotations(#{submission.id}, #{submission.course_id.to_json}, #{submission.exercise_id}, #{user.id}, #{@code.to_json}, #{user_is_student});
window.dodona.codeListing.initAnnotations(#{submission.id}, #{submission.course_id.to_json}, #{submission.exercise_id}, #{user.id}, #{@code.to_json}, #{user_is_student}, #{can_submit_as_own});
window.dodona.codeListing.addMachineAnnotations(#{messages.to_json});
#{'window.dodona.codeListing.initAnnotateButtons();' if user_perm}
#{'window.dodona.codeListing.loadUserAnnotations();' if submission.annotated? || (!user_is_student && submission.annotations.any?)}
Expand Down
2 changes: 1 addition & 1 deletion app/policies/submission_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def evaluate?
end

def create?
user
user.present?
end

def edit?
Expand Down
15 changes: 8 additions & 7 deletions app/views/feedbacks/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,14 @@
<div class="col-12 order-lg-0 col-lg-8 col-xxl-9 feedback-submission">
<div class="card feedback-show">
<div class="card-title card-title-colored">
<h2 class="card-title-text"><span>
<%= t '.submission_for' %>
<%= link_to(@feedback.exercise.name, course_series_activity_path(@feedback.evaluation.series.course, @feedback.evaluation.series, @feedback.exercise)) %>
<%= t '.by' %>
<%= link_to(@feedback.user.full_name, course_member_path(@feedback.evaluation.series.course, @feedback.user)) %>
</span></h2>
<%= %>
<h2 class="card-title-text">
<span>
<%= t '.submission_for' %>
<%= link_to(@feedback.exercise.name, course_series_activity_path(@feedback.evaluation.series.course, @feedback.evaluation.series, @feedback.exercise)) %>
<%= t '.by' %>
<%= link_to(@feedback.user.full_name, course_member_path(@feedback.evaluation.series.course, @feedback.user)) %>
</span>
</h2>
</div>
<div class="card-supporting-text card-border">
<div class="row">
Expand Down
2 changes: 1 addition & 1 deletion app/views/submissions/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<div class="card-title card-title-colored">
<h2 class="card-title-text"><%= t ".submission_results" %></h2>
<div class="card-title-fab">
<% if policy(@submission).create? %>
<% if policy(@submission).create? && @submission.user == current_user %>
<%= link_to edit_submission_path(@submission), class: 'btn btn-fab', title: t('.edit') do %>
<i class="mdi mdi-square-edit-outline"></i>
<% end %>
Expand Down
3 changes: 3 additions & 0 deletions config/locales/js/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -325,3 +325,6 @@ en:
dark: Dark
system: System
theme: Theme
feedbacks:
submission:
submit: Submit this solution
3 changes: 3 additions & 0 deletions config/locales/js/nl.yml
Original file line number Diff line number Diff line change
Expand Up @@ -325,3 +325,6 @@ nl:
dark: Donker
system: Systeem
theme: Stijl
feedbacks:
submission:
submit: Deze oplossing indienen
2 changes: 1 addition & 1 deletion test/javascript/code_listing.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ beforeEach(async () => {
</table>
</div>
</div>`);
codeListing.initAnnotations(54, 1, 1, 1, "print(5 + 6)\nprint(6 + 3)\nprint(9 + 15)\n");
codeListing.initAnnotations(54, 1, 1, 1, "print(5 + 6)\nprint(6 + 3)\nprint(9 + 15)\n", false, false);
annotationState.visibility = "all";
userAnnotationState.reset();
machineAnnotationState.setMachineAnnotations([]);
Expand Down