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

Detect and ignore duplicate submissions #1006

Closed
PasiSa opened this issue Mar 24, 2022 · 11 comments · Fixed by #1048
Closed

Detect and ignore duplicate submissions #1006

PasiSa opened this issue Mar 24, 2022 · 11 comments · Fixed by #1048
Assignees
Labels
area: exercises Related to exercises. Use other labels to focus issue to UI/backend area: UX student User experience and usability for students effort: hours Estimated to take less than one day, from the creation of a new branch to the merging experience: beginner required knowledge estimate requester: CS The issue is raised internally by a CS teacher requires: priority Currently using this label to flag issues that need EDIT decision ASAP (even if there was priority) type: feature New feature or change to a feature
Milestone

Comments

@PasiSa
Copy link
Contributor

PasiSa commented Mar 24, 2022

User may accidentally make a submission that is exact duplicate of an earlier submission for the same exercise. A+ could detect and ignore these submissions, to avoid unnecessary consuming available submissions for the exercise.

@PasiSa PasiSa added type: feature New feature or change to a feature area: UX student User experience and usability for students effort: hours Estimated to take less than one day, from the creation of a new branch to the merging area: exercises Related to exercises. Use other labels to focus issue to UI/backend experience: beginner required knowledge estimate requester: CS The issue is raised internally by a CS teacher requires: priority Currently using this label to flag issues that need EDIT decision ASAP (even if there was priority) labels Mar 24, 2022
@markkuriekkinen
Copy link
Member

markkuriekkinen commented Mar 30, 2022

Duplicate of #513. There are some concerns there:

UI should show warning when resubmitting an earlier answer to an exercise

Student should be warned when they are about to consume an attemt for already submitted solution.

A+ could resubmit the older submission for regrading, which would enable them to fix failed gradings them selfs, which does raise concerns for unlimited code executions in the backend.

This could be implemented by calculating a hash value of the submission (all fields and files, sorted), exposing this hash back to the browser, which could compare the current submission to past hashes.

@markkuriekkinen
Copy link
Member

Ideas:

  • compute a hash of the submission so that the hashes can be compared. Identical hashes suggest that the submissions are identical.
  • the hash should be computed over submitted files and Submission.submission_data
  • the hash can be stored in the database in the field Submission.meta_field. It is a JSON field that contains other meta data as well.
  • the hash for each submission can be added to the cache (CachedPoints) so that reading the hashes of all submissions from one student is fast (no database queries if the data is already in the cache).
    • entry['submissions'].append({
      'type': 'submission',
      'id': submission.id,
      'max_points': entry['max_points'],
      'points_to_pass': entry['points_to_pass'],
      'confirm_the_level': entry.get('confirm_the_level', False),
      'submission_count': 1, # to fool points badge
      'points': submission.grade,
      'formatted_points': format_points(submission.grade, True, False),
      'graded': submission.is_graded, # TODO: should this be official (is_graded = ready or unofficial)
      'passed': (submission.grade >= entry['points_to_pass']),
      'submission_status': submission.status if not submission.is_graded else False,
      'unofficial': unofficial,
      'date': submission.submission_time,
      'url': submission.get_url('submission-plain'),
      'feedback_revealed': True,
      'feedback_reveal_time': None,
      })

@PasiSa PasiSa added this to the v1.15 milestone Apr 11, 2022
@markkuriekkinen
Copy link
Member

Additional ideas for the implementation.

  1. CachedPoints: creating a CachedPoints objects reads data from the cache or generates the data if it is missing from the cache. The constructor is here:
    def __init__(
    self,
    course_instance: CourseInstance,
    user: User,
    content: CachedContent,
    is_staff: bool = False,
    ) -> None:
  • As an example, the constructor is called in many places in the existing code including these:
  • points = CachedPoints(self.instance, request.user, self.content, self.is_course_staff)
  • cache = CachedPoints(self.instance, user or self.request.user, self.content, self.is_course_staff)
  • points = CachedPoints(module.course_instance, request.user, view.content, view.is_course_staff)
  • the CachedContent needed as a parameter usually already exists in the view classes as self.content:
    self.content = CachedContent(self.instance)
  • The next step is to find the correct data inside the cache object.
  • The cache object has a method find():
    def find(self, model):
  • The parameter to find() could a BaseExercise instance:
    elif isinstance(model, LearningObject):
  • find() returns a tuple. The first element is the dictionary for the exercise and it contains the key "submissions"
  1. when and where to check duplicates: it is best to avoid creating the real submission object in the database if the submission should be declined due to the duplicate check. The duplicate check can be done with the data in the http request even before creating a new submission. The duplicate check could be implemented in a new method in the class SubmissionManager:
    class SubmissionManager(JWTAccessible["Submission"], models.Manager):
  • ExerciseView could call the duplicate check method before creating the new submission. The existing submission_check method is appropriate for this:
    def submission_check(self, error=False, request=None):
  • submission_check returns a list of warnings (amongst other values) and an error message about the duplicate submission should be included there.
  1. how to show warnings to the user: if you use the submission_check method from the bullet 2 above, then submission_check returns a list of warnings that are displayed at the top of the exercise. That would follow the same approach that has been used with existing warnings in making submissions.

@ttsirkia
Copy link
Contributor

ttsirkia commented May 12, 2022

How to handle the situations that the grader has had some bugs which are fixed and after that student would like to submit the same solution again?

@markkuriekkinen
Copy link
Member

How to handle the situations that the grader has had some bugs which are fixed and after that student would like to submit the same solution again?

The student is shown a confirmation dialog about whether he/she wants to submit a new identical submission.

@ttsirkia
Copy link
Contributor

That makes sense. So the title of the issue is not accurate. Duplicates are not ignored, they are confirmed.

@markkuriekkinen
Copy link
Member

I think I wrote some parts in the earlier comments from the perspective that duplicate submissions would always be rejected, but we are supposed to offer the student a confirmation dialog if they want to submit the duplicate or not.

  • The confirmation dialog needs to be implemented in the frontend (JavaScript).
  • The code chapter.js already uses AJAX to submit the solution. It's possible to open a new modal dialog there at the right time.
  • The submission needs to be sent to the server so that the server can check if there are duplicates. If there are duplicates, then the server doesn't create a new submission and returns a message to the browser that the duplicate confirmation is needed.
  • If the user wants to make a duplicate, then the submission is again sent to the server. There could be a GET query parameter in the URL so that the server accepts the duplicate and creates the submission as usual. The submission is then sent to grading and so on.

@markkuriekkinen
Copy link
Member

@muhammadfaiz12 I think implementing the confirmation dialog in the frontend is much simpler if the duplicate check is actually done in the frontend. Then, duplicate submissions would never be uploaded to the server. If the hashes of the existing submissions are rendered in the HTML (data attributes), then frontend JavaScript can compute the hash for the new submission and compare the hash to the existing hashes. If it is a duplicate and the student wants to cancel the submission in the dialog, then no submission is ever sent to the server. This just requires that the hashes can be computed in frontend JavaScript in addition to the backend.

@muhammadfaiz12
Copy link
Contributor

Yes that could also works but i think i need more explanation on how this would work. We could discuss this in the meeting.

@muhammadfaiz12
Copy link
Contributor

Handover Note

What is this and how it works?
This feature check if a user will make a duplicate submission. If the system detects that the user will make a duplicate submission it will prompt a confirmation from the user. User would be able to submit the submission if they indicate so in the confirmation. No changes of behaviour happened if the user make a non-duplicate submission.

The system create a submission hash for submitted submission. this hash was stored in both Database and Cache. This hashes are rendered to Front end when user open exercise page. When user want to submit a submission, the front end system create hash of the submission and compare it to the old submissions hashes. Front end side create hash to compare new submission hash with the previous submissions hashes, Back end side then recompute the hash to store the hash in the database. The FE created hash and BE created hash has to be the same (as per now, it still different)

The feature description are also already written on this thread and on the PR

Things To Do
While the majority of the feature parts are already completed, several things to do are still present.

  • Create modal for submission reconfirmation dialogue
  • Ensure Hash created in FE are the same with hash created in BE for the same submission
  • Hashing for File Submission. There could be some work needed with making the file can be hashed with the md5 algorithm.

Overall testing are also need to be done, especially after the points above are completed.

Note
The source code contains extensive comments in the related parts of code, developer could refer to that comment if they are confused about what the code are meant to do. To summarize, most of FE code are on exercise/static/chapter.js while backend code are in submission_models.py, exercise/cache/points.py and lib/helpers.py. As we can see, the bulk of the logical operations are mainly on front end side while the backend side only handles the creation of hash for the storage purposes.

@markkuriekkinen
Copy link
Member

Comments about modal dialogs in chapter.js:

  • the html code for the modal has been included in all pages in the base html template:

    <div id="page-modal" class="modal" role="dialog">

  • chapter.js finds the modal html structure in the initialization:

    this.modalElement = $(this.settings.modal_selector);

  • the jQuery method aplusModal() is our extension defined here:

    a-plus/assets/js/aplus.js

    Lines 362 to 452 in 96a1350

    (function($) {
    "use strict";
    const pluginName = "aplusModal";
    var defaults = {
    loader_selector: ".modal-progress",
    loader_text_selector: ".progress-bar",
    title_selector: ".modal-title",
    content_selector: ".modal-body",
    error_message_attribute: "data-msg-error",
    };
    function AplusModal(element, options) {
    this.element = $(element);
    this.settings = $.extend({}, defaults, options);
    this.init();
    }
    $.extend(AplusModal.prototype, {
    init: function() {
    this.loader = this.element.find(this.settings.loader_selector);
    this.loaderText = this.loader.find(this.settings.loader_text_selector);
    this.title = this.element.find(this.settings.title_selector);
    this.content = this.element.find(this.settings.content_selector);
    this.messages = {
    loading: this.loaderText.text(),
    error: this.loaderText.attr(this.settings.error_message_attribute)
    };
    },
    run: function(command, data) {
    switch (command) {
    case "open":
    this.open(data);
    break;
    case "error":
    this.showError(data);
    break;
    case "content":
    this.showContent(data);
    break;
    }
    },
    open: function(data) {
    this.title.hide();
    this.content.hide();
    this.loaderText
    .removeClass('progress-bar-danger').addClass('active')
    .text(data || this.messages.loading);
    this.loader.show();
    this.element.on("hidden.bs.modal", function(event) {
    $(".dropdown-toggle").dropdown();
    });
    this.element.modal("show");
    },
    showError: function(data) {
    this.loaderText
    .removeClass('active').addClass('progress-bar-danger')
    .text(data || this.messages.error);
    },
    showContent: function(data) {
    this.loader.hide();
    if (data.title) {
    this.title.text(data.title);
    this.title.show();
    }
    if (data.content instanceof $) {
    this.content.empty().append(data.content);
    } else {
    this.content.html(data.content);
    }
    this.content.show();
    return this.content;
    }
    });
    $.fn[pluginName] = function(command, data, options) {
    return this.each(function() {
    var modal = $.data(this, "plugin_" + pluginName);
    if (!modal) {
    modal = new AplusModal(this, options);
    $.data(this, "plugin_" + pluginName, modal);
    }
    return modal.run(command, data);
    });
    };
    })(jQuery);

  • the function modal() is part of the Bootstrap API for opening modal dialogs from JS code:

    this.element.modal("show");

  • https://getbootstrap.com/docs/3.4/javascript/#via-javascript

@markkuriekkinen markkuriekkinen modified the milestones: v1.15, v1.16 Jun 10, 2022
ihalaij1 pushed a commit to ihalaij1/a-plus that referenced this issue Aug 9, 2022
ihalaij1 added a commit to ihalaij1/a-plus that referenced this issue Aug 9, 2022
markkuriekkinen pushed a commit that referenced this issue Aug 9, 2022
murhum1 pushed a commit to murhum1/a-plus that referenced this issue Nov 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: exercises Related to exercises. Use other labels to focus issue to UI/backend area: UX student User experience and usability for students effort: hours Estimated to take less than one day, from the creation of a new branch to the merging experience: beginner required knowledge estimate requester: CS The issue is raised internally by a CS teacher requires: priority Currently using this label to flag issues that need EDIT decision ASAP (even if there was priority) type: feature New feature or change to a feature
Projects
Status: Done
5 participants