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

Submission Version Switching #1503

Merged
merged 26 commits into from
Jul 24, 2022
Merged

Submission Version Switching #1503

merged 26 commits into from
Jul 24, 2022

Conversation

damianhxy
Copy link
Member

@damianhxy damianhxy commented Apr 20, 2022

Description

  • Add buttons to switch to prev / next version of a student's submission that contains a specific file
  • Make buttons update when switching between files

Motivation and Context

When investigating AIVs, instructors might want to track changes to a specific file across the different versions of a student's submission. This PR adds buttons to enable instructors to easily switch between the different versions.

Note: For single file handin formats, the arrows will effectively work to cycle through the versions.

How Has This Been Tested?

  • Create a new assessment with handin format handin.tar
  • Submit hello.tar, as well as modified versions (e.g. remove files, add files)
  • Ensure that prev / next buttons work as expected (i.e. that they go to the same file, skip versions where the file doesn't exist)
  • Ensure that when we change the file selected, the buttons update as expected
  • Also check that for single file handin formats (e.g. .c, .pdf) that the arrow buttons serve to enumerate through the versions

Screen Shot 2022-07-17 at 20 46 57

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have run rubocop for style check. If you haven't, run overcommit --install && overcommit --sign to use pre-commit hook for linting
  • My change requires a change to the documentation, which is located at Autolab Docs
  • I have updated the documentation accordingly, included in this PR

@damianhxy damianhxy force-pushed the fast-version-switching branch from 7b07d12 to 03da4d9 Compare April 26, 2022 19:18
@damianhxy damianhxy marked this pull request as ready for review April 26, 2022 21:14
@damianhxy damianhxy marked this pull request as draft April 27, 2022 03:05
@damianhxy damianhxy mentioned this pull request Apr 27, 2022
6 tasks
@damianhxy damianhxy closed this Apr 27, 2022
@damianhxy damianhxy reopened this Apr 29, 2022
@damianhxy damianhxy marked this pull request as ready for review April 29, 2022 03:44
@damianhxy damianhxy marked this pull request as draft May 21, 2022 02:44
@damianhxy damianhxy changed the title Fast Version Switching Submission File Version Switching Jul 10, 2022
@damianhxy damianhxy marked this pull request as ready for review July 17, 2022 10:00
@damianhxy damianhxy changed the title Submission File Version Switching Submission Version Switching Jul 17, 2022
@victorhuangwq
Copy link
Contributor

@damianhxy could you elaborate what was changed since we last discussed?

At the same time I do have concerns that people might get confused by the [# student / total students] and version / # of versions

image

Copy link
Contributor

@victorhuangwq victorhuangwq left a comment

Choose a reason for hiding this comment

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

Functionally it looks good to me as described. I would just like some clarification and I think we are good to go.

app/views/submissions/view.html.erb Outdated Show resolved Hide resolved
@damianhxy
Copy link
Member Author

@damianhxy could you elaborate what was changed since we last discussed?

The main difference is that the arrow buttons now work on a per-file basis.

Previously, they would simply bring you to the next or previous version (and reset you to header_position 0). They would be grayed out when you are on the first / last version. But as Iliano pointed out, that isn't very useful from an instructor's perspective.

Now, the arrow buttons operate on a per-file basis. That is to say, clicking them will bring you to the next or previous version that contains the file (i.e. the same filepath). They will also now be grayed out when there is no older / newer version containing the file.

How this works is that when viewing a file (e.g. from an archive), we look through the other versions and search each one for the same filepath. If it exists, we take note of the header_position of the file in that particular version and append it to the querystring of the buttons so that we will jump directly to the same file when switching versions.

Note however that the functionality of the version dropdown remains unchanged, and using it to change versions will reset you to header_position 0 as usual.

At the same time I do have concerns that people might get confused by the [# student / total students] and version / # of versions

image

Perhaps we can attach a title to the span so that users will see explanatory text when they hover over it?

Screen Shot 2022-07-22 at 13 01 15

@victorhuangwq
Copy link
Contributor

I think adding the title would address the issue - good enough fix for now, because I think we might want to reimagine this page in the future.

At the same time I tested this

  • Ensure that prev / next buttons work as expected (i.e. that they go to the same file, skip versions where the file doesn't exist)

It works as mentioned. It doesn't skip if I submitted a single file, and then a tar file afterwards however. But I guess that's an okay behavior. (i.e. hello.c, then i submit hello.tar containing abc.c & def.c)

@damianhxy
Copy link
Member Author

It works as mentioned. It doesn't skip if I submitted a single file, and then a tar file afterwards however. But I guess that's an okay behavior. (i.e. hello.c, then i submit hello.tar containing abc.c & def.c)

Yup, I don't expect the handin format to change across versions after all

Copy link
Contributor

@victorhuangwq victorhuangwq left a comment

Choose a reason for hiding this comment

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

LGTM

@damianhxy damianhxy merged commit 8415704 into master Jul 24, 2022
@damianhxy damianhxy deleted the fast-version-switching branch July 24, 2022 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants