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.

Sign up for GitHub

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

Jump to bottom

[#10593] Disable view/download session results button according to instructor privilege #10611

Merged
merged 3 commits into from
Aug 17, 2020
Merged

[#10593] Disable view/download session results button according to instructor privilege #10611

merged 3 commits into from
Aug 17, 2020

Conversation

niqiukun
Copy link
Contributor

Fixes #10593

image

@niqiukun niqiukun added the s.ToReview The PR is waiting for review(s) label Aug 16, 2020
wkurniawan07
wkurniawan07 reviewed Aug 16, 2020
View reviewed changes
src/web/app/components/sessions-table/sessions-table.component.html Outdated
@@ -86,7 +86,8 @@
<div ngbDropdown class="d-inline-block">
<button class="btn btn-light btn-sm" ngbDropdownToggle>Results</button>
<div ngbDropdownMenu>
<a class="btn dropdown-item clickable" (click)="viewSessionResultEvent.emit(idx)"
<a class="btn dropdown-item clickable {{!sessionsTableRowModel.instructorPrivilege.canViewSessionInSections ? 'disabled' : ''}}"
Copy link
Member

Choose a reason for hiding this comment

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

Use ngClass instead?

@madanalogy
Copy link
Contributor

I think it might be better to wait for some resolution in #10613 before proceeding with this PR. The required permission for access that you outlined here is canViewSessionInSections but in the backend currently, using GET on /session endpoint requires canSubmitSessionInSections.

@madanalogy madanalogy added the s.OnHold The issue/PR's validity has been put on hold pending some other event label Aug 17, 2020
@wkurniawan07
Copy link
Member

using GET on /session endpoint requires canSubmitSessionInSections

This does not sound right. This PR in fact does the right thing.

wkurniawan07
wkurniawan07 approved these changes Aug 17, 2020
View reviewed changes
Copy link
Member

@wkurniawan07 wkurniawan07 left a comment

Choose a reason for hiding this comment

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

LGTM

@wkurniawan07 wkurniawan07 added s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging and removed s.OnHold The issue/PR's validity has been put on hold pending some other event s.ToReview The PR is waiting for review(s) labels Aug 17, 2020
@madanalogy madanalogy mentioned this pull request Aug 17, 2020
Closed
@madanalogy madanalogy merged commit 8765a96 into TEAMMATES:master Aug 17, 2020
@madanalogy madanalogy added this to the v7.0.1 milestone Aug 22, 2020
@madanalogy madanalogy added the c.Feature User-facing feature; can be new feature or enhancement to existing feature label Aug 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wkurniawan07 wkurniawan07 wkurniawan07 approved these changes

Assignees

@wkurniawan07 wkurniawan07

Labels
c.Feature User-facing feature; can be new feature or enhancement to existing feature s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging
Projects
None yet
Milestone
V7.0.1
Development

Successfully merging this pull request may close these issues.

Instructor session result: empty csv downloaded for instructors without permission
3 participants
@niqiukun @madanalogy @wkurniawan07