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

Hide download handout button if assessement has no handout #1951

Merged
merged 3 commits into from
Aug 2, 2023

Conversation

damianhxy
Copy link
Member

@damianhxy damianhxy commented Jul 20, 2023

Summary

Summary generated by Reviewpad on 02 Aug 23 05:00 UTC

This pull request includes two patches.

The first patch hides the "handout" button if no handout is defined. It makes changes to the handout controller, assessment model, and the show.html.erb view file.

The second patch provides a temporary fix for a chromedriver issue. It adds a fix to the rails_helper.rb file to address a specific version issue with chromedriver.

Please review these changes and test them accordingly.

Description

Hide "download handout" button if assessment does not overwrite handout method, and handout is neither a url nor a file.

Motivation and Context

Removes non-applicable menu items to avoid clutter and confusion. (similar logic for the writeup button was applied in #1615)

Closes #1941

How Has This Been Tested?

  • Create a new assessment, observe that "download handout" button is absent
  • Set handout to be a random string, observe that "download handout" button is still absent
  • Set handout to be a url, observe that "download handout" button appears and works
  • Set handout to be a file (e.g. <asmt>.yml), observe that "download handout" button appears and works
  • Clear value of handout, define handout method in .rb file as follows:
def handout
    {
      "fullpath" => "<full path to a file>",
      "filename" => "<file name>"
    }
end

Reload config file. Observe that "download handout" button appears and works.

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

@reviewpad reviewpad bot added the small Pull request is small label Jul 20, 2023
@damianhxy damianhxy marked this pull request as ready for review July 20, 2023 18:22
@reviewpad reviewpad bot requested a review from evanyeyeye July 20, 2023 18:22
@KesterTan
Copy link
Contributor

Everything works as expected. LGTM!

@damianhxy damianhxy added this pull request to the merge queue Aug 1, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 1, 2023
@damianhxy damianhxy added this pull request to the merge queue Aug 2, 2023
Merged via the queue into master with commit 6bd7bc6 Aug 2, 2023
6 checks passed
@damianhxy damianhxy deleted the hide-handout-button branch August 2, 2023 05:11
NicholasMy pushed a commit to UB-CSE-IT/Autolab that referenced this pull request Jan 4, 2024
)

* Hide handout button if no handout defined

* Temporary fix for chromedriver issue

(cherry picked from commit 6bd7bc6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
small Pull request is small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hide handout button when assessment has no handout
2 participants