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

Fix for Issue 1728 #1750

Merged
merged 8 commits into from Sep 20, 2014

Conversation

Projects
None yet
2 participants
@bmuscede
Contributor

bmuscede commented Sep 12, 2014

ISSUE: #1728

DESCRIPTION: If you try to CSV-upload a hidden assignment, the assignment isn't created properly. Additionally, if you make an assignment hidden, the assignment sidebar does not work properly. It appears to be created in the database, and appears in the table, but trying to access it raises an error.

FIX: The link generation methods from _assignments_dropdown_menu.html.erb to create the hidden links were not used properly. These would result in error when the sidebar was generated and cause assignments to be unreachable. Instead, the controller and action sections on lines 97 and 98 were switched to point to the correct page and to have the correct action. Also, on lines 121 - 123 a check was inserted to see if the current assignment is hidden. If so, the assignment sidebar will display that the assignment is hidden.

TESTING: Various hidden assignments were added and tested to ensure that all assignments were still reachable. Additionally, previously created assignments were set to hidden to see if TAs and the admin could still access all assignments.

@bmuscede bmuscede changed the title from Issue 1728 to Fix for Issue 1728 Sep 12, 2014

@david-yz-liu

This comment has been minimized.

Show comment
Hide comment
@david-yz-liu

david-yz-liu Sep 15, 2014

Contributor

Hey @bmuscede, I don't think 'edit' should be hardcoded in. If I'm browsing the Submissions page for an assignment, and I use the sidebar to go to a different assignment, I should end up in the Submissions page for that assignment, not its edit page. This the current behaviour for non-hidden assignments.

Not sure if this breaks the fix, but it really should be the way these links work.

Contributor

david-yz-liu commented Sep 15, 2014

Hey @bmuscede, I don't think 'edit' should be hardcoded in. If I'm browsing the Submissions page for an assignment, and I use the sidebar to go to a different assignment, I should end up in the Submissions page for that assignment, not its edit page. This the current behaviour for non-hidden assignments.

Not sure if this breaks the fix, but it really should be the way these links work.

@bmuscede

This comment has been minimized.

Show comment
Hide comment
@bmuscede

bmuscede Sep 15, 2014

Contributor

I will definitely take a look at that. If I remove the hardcoded edit part, it seems to break the fix. It must be an issue with how the controller.action_name is assigned.
I should be able to have a better fix in by tonight or early tomorrow that does not have anything hardcoded.

Contributor

bmuscede commented Sep 15, 2014

I will definitely take a look at that. If I remove the hardcoded edit part, it seems to break the fix. It must be an issue with how the controller.action_name is assigned.
I should be able to have a better fix in by tonight or early tomorrow that does not have anything hardcoded.

@bmuscede

This comment has been minimized.

Show comment
Hide comment
@bmuscede

bmuscede Sep 16, 2014

Contributor

Here's the re-upload with the better fix.

ISSUE: #1728

DESCRIPTION: If you try to CSV-upload a hidden assignment, the assignment isn't created properly. Additionally, if you make an assignment hidden, the assignment sidebar does not work properly. It appears to be created in the database, and appears in the table, but trying to access it raises an error.

FIX: Two things were changed in this re-upload. To start, in _sub_menu.html.erb, a check was added to see if the currently selected assignment is hidden. If hidden, a "(hidden)" is appended to the end of the sidebar. This allows for admins and TAs to see that their current assignment is hidden. Finally, in _assignments_dropdown_menu.html.erb, a fix was made on lines 97-99 to use the polymorphic_url method to generate the links for hidden assignments. This ensures that switching between assignments still takes users to the correct sections of the website.

TESTING: Two assignments were created and made hidden and then all the assignment sidebar links were tested. Additionally, a third assignment was created and set to "not hidden" to ensure non-hidden assignments worked. All sidebar links were tested.

Contributor

bmuscede commented Sep 16, 2014

Here's the re-upload with the better fix.

ISSUE: #1728

DESCRIPTION: If you try to CSV-upload a hidden assignment, the assignment isn't created properly. Additionally, if you make an assignment hidden, the assignment sidebar does not work properly. It appears to be created in the database, and appears in the table, but trying to access it raises an error.

FIX: Two things were changed in this re-upload. To start, in _sub_menu.html.erb, a check was added to see if the currently selected assignment is hidden. If hidden, a "(hidden)" is appended to the end of the sidebar. This allows for admins and TAs to see that their current assignment is hidden. Finally, in _assignments_dropdown_menu.html.erb, a fix was made on lines 97-99 to use the polymorphic_url method to generate the links for hidden assignments. This ensures that switching between assignments still takes users to the correct sections of the website.

TESTING: Two assignments were created and made hidden and then all the assignment sidebar links were tested. Additionally, a third assignment was created and set to "not hidden" to ensure non-hidden assignments worked. All sidebar links were tested.

Show outdated Hide outdated app/views/shared/_assignments_dropdown_menu.html.erb
<span title='<%= assignment.description %>'>
<%= t('assignment.hidden',
assignment_text: h(assignment.short_identifier)) %>
</span>

This comment has been minimized.

@david-yz-liu

david-yz-liu Sep 19, 2014

Contributor

I think the span tags aren't aligned properly. Also, wouldn't it make more sense to put the if block into the span?

@david-yz-liu

david-yz-liu Sep 19, 2014

Contributor

I think the span tags aren't aligned properly. Also, wouldn't it make more sense to put the if block into the span?

david-yz-liu pushed a commit that referenced this pull request Sep 20, 2014

@david-yz-liu david-yz-liu merged commit 8191ce8 into MarkUsProject:master Sep 20, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment