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

General: Do not show certain buttons for tutors in lecture overview and course management #7423

Merged

Conversation

JohannesStoehr
Copy link
Contributor

@JohannesStoehr JohannesStoehr commented Oct 19, 2023

Checklist

General

Client

  • Important: I implemented the changes with a very good performance, prevented too many (unnecessary) REST calls and made sure the UI is responsive, even with large data.
  • I followed the coding and design guidelines.
  • I added multiple integration tests (Jest) related to the features (with a high test coverage), while following the test guidelines.
  • I added authorities to all new routes and checked the course groups for displaying navigation elements (links, buttons).
  • I added multiple screenshots/screencasts of my UI changes.

Motivation and Context

Fixes #7422 and also some other occurrences I noticed when fixing this issue.

Description

Fixes authorities and hides the following buttons from tutors:

  • Manage button in the lecture view inside a course (the one from Lecture: lecture unit creation page visible even if no permission #7422)
  • "Edit"/"Attachment"/"Units" buttons in the manage page of a lecture
  • Scores button in the course management tab bar
  • In the general course management the "Tutorials" button in the course card, if there is no tutorial configuration yet

Also shows the tutorials button in the course management tab bar of a specific course so tutors have access to their tutor groups and can sign up students. This is only possible after an instructor has configured the tutorial groups.

Steps for Testing

Prerequisites:

  • 1 Tutor
  • 1 Editor/Instruktor
  1. Check that the buttons are successfully hidden from tutors in a course without a tutorial group configuration
  2. Check that instructors see all buttons
  3. Enable tutorial groups (the tutorials tab guides the instructor through this) or find a course with tutorial groups enabled
  4. Check that tutors can now access tutorials in the course management
    (Also see attached screenshots and links to examples on TS1)

Testserver States

Note

These badges show the state of the test servers.
Green = Currently available, Red = Currently locked







Review Progress

Performance Review

  • I (as a reviewer) confirm that the client changes (in particular related to REST calls and UI responsiveness) are implemented with a very good performance

Code Review

  • Code Review 1
  • Code Review 2

Manual Tests

  • Test 1
  • Test 2

Screenshots

The hidden Buttons:

Manage button, e.g. here: https://artemis-test1.artemis.in.tum.de/courses/118/lectures/215
Bildschirmfoto 2023-10-19 um 17 04 08

"Edit"/"Attachment"/"Units" buttons, e.g. here: https://artemis-test1.artemis.in.tum.de/course-management/118/lectures/215
Bildschirmfoto 2023-10-19 um 17 04 19

Scores button and tutorials button not visible to tutors, e.g. here: https://artemis-test1.artemis.in.tum.de/course-management/118
Bildschirmfoto 2023-10-21 um 11 33 25

Tutorials button visible in tab bar, e.g. here: https://artemis-test1.artemis.in.tum.de/course-management/118
Bildschirmfoto 2023-10-21 um 11 34 59

Tutorials button visible in course management card, e.g. here: https://artemis-test1.artemis.in.tum.de/course-management under the test course section
Bildschirmfoto 2023-10-21 um 11 35 37

@JohannesStoehr JohannesStoehr added ready for review client Pull requests that update TypeScript code. (Added Automatically!) small bugfix user interface labels Oct 19, 2023
@JohannesStoehr JohannesStoehr added this to the 6.6.2 milestone Oct 19, 2023
@JohannesStoehr JohannesStoehr self-assigned this Oct 19, 2023
@JohannesStoehr JohannesStoehr requested a review from a team as a code owner October 19, 2023 15:06
@github-actions github-actions bot added the tests label Oct 19, 2023
konrad2002
konrad2002 previously approved these changes Oct 19, 2023
Copy link
Member

@konrad2002 konrad2002 left a comment

Choose a reason for hiding this comment

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

LGTM
Tested on Legacy TS2

Strohgelaender
Strohgelaender previously approved these changes Oct 19, 2023
Copy link
Contributor

@Strohgelaender Strohgelaender left a comment

Choose a reason for hiding this comment

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

code lgtm, thanks for adding a small additional client test

Copy link
Collaborator

@MaximilianAnzinger MaximilianAnzinger left a comment

Choose a reason for hiding this comment

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

Code lgtm

dearjasmina
dearjasmina previously approved these changes Oct 20, 2023
Copy link
Contributor

@dearjasmina dearjasmina left a comment

Choose a reason for hiding this comment

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

tested on ts1, works as described for both tutor and instructor roles 👍🏻

…urse-manage-authorities' into bugfix/fix-manage-lecture-and-course-manage-authorities
JanaNF
JanaNF previously approved these changes Oct 20, 2023
Copy link

@JanaNF JanaNF left a comment

Choose a reason for hiding this comment

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

Tested on ts1. All good

Copy link
Member

@konrad2002 konrad2002 left a comment

Choose a reason for hiding this comment

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

tested it on legacy ts11

I'm not sure if it is supposed to be like this, but as a tutor I wasn't able to open the tutorial groups page..

As instructor this worked

Tutor:

grafik
When clicking, nothing happens (+ no error in console)

Instructor:

grafik
After clicking on tutorials

@@ -254,7 +254,7 @@ <h4 class="text-center no-exercises mt-3 fw-medium">{{ 'artemisApp.course.noExer
<span class="d-none d-xl-inline">{{ 'artemisApp.courseOverview.menu.messages' | artemisTranslate }}</span>
</a>
<a
*ngIf="course.isAtLeastTutor"
*ngIf="course.timeZone || course.isAtLeastInstructor"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We currently don't load the tutorialGroupsConfiguration here and to keep this PR small I decided to only use the timeZone here, which is also a prerequisite for tutorials

@JohannesStoehr JohannesStoehr changed the title General: Hide unavailable buttons from tutors in lecture overview and course management General: Fix button authorities for tutors in lecture overview and course management Oct 21, 2023
@krusche krusche modified the milestones: 6.6.2, 6.6.3 Oct 21, 2023
Copy link
Collaborator

@MaximilianAnzinger MaximilianAnzinger left a comment

Choose a reason for hiding this comment

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

Code

Copy link

@JanaNF JanaNF left a comment

Choose a reason for hiding this comment

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

Manual tested on legacy ts2. Worked as expected... again ;)

Copy link
Contributor

@sarpsahinalp sarpsahinalp left a comment

Choose a reason for hiding this comment

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

Tested on ts3 works as expected.
Tutor side:
image

Instructor side:
Screenshot from 2023-10-24 19-02-33

@krusche krusche changed the title General: Fix button authorities for tutors in lecture overview and course management General: Do not show certain buttons for tutors in lecture overview and course management Oct 26, 2023
@krusche krusche merged commit 2f5d20c into develop Oct 26, 2023
37 of 40 checks passed
@krusche krusche deleted the bugfix/fix-manage-lecture-and-course-manage-authorities branch October 26, 2023 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix client Pull requests that update TypeScript code. (Added Automatically!) ready to merge small tests user interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lecture: lecture unit creation page visible even if no permission
8 participants