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

Development: Refactor isAtLeast occurrences in client and unify spy testing #5507

Merged
merged 29 commits into from
Sep 13, 2022

Conversation

JohannesStoehr
Copy link
Contributor

@JohannesStoehr JohannesStoehr commented Jul 29, 2022

Checklist

General

Client

Motivation and Context

#3742 refactored the client code such that exercise/course objects store the current access rights in the client. There were not used in some components.

Additionally GitHub was complaining about the checking of some spies in the client. These are fixed and unified in the code as well. (Point 11: https://docs.artemis.ase.in.tum.de/dev/guidelines/client-tests/)

Description

This PR refactores the last components to also use the exercise/course objects for the access rights.
The occurrences using the Metis service (e.g. post-header.component) is intentionally left out.

For checking the spies, we use .toHaveBeenCalledOnce()/.toHaveBeenCalledTimes(...) instead of .toHaveBeenCalled() and .not.toHaveBeenCalled() instead of .toHaveBeenCalled(0). This is unified in this PR

Review Progress

Code Review

  • Review 1
  • Review 2

@github-actions github-actions bot added the client Pull requests that update TypeScript code. (Added Automatically!) label Jul 29, 2022
@github-actions github-actions bot added the tests label Jul 29, 2022
@JohannesStoehr JohannesStoehr marked this pull request as ready for review July 29, 2022 17:16
@JohannesStoehr JohannesStoehr requested a review from a team as a code owner July 29, 2022 17:16
@krusche krusche added this to the 5.9.6 milestone Jul 29, 2022
@krusche krusche changed the title 'Development': Refactor isAtLeast occurrences in client Development: Refactor isAtLeast occurrences in client Jul 29, 2022
…ences

# Conflicts:
#	src/main/webapp/app/exercises/shared/dashboards/tutor/exercise-assessment-dashboard.component.html
@krusche krusche modified the milestones: 5.9.6, 5.9.7 Jul 31, 2022
ge65cer
ge65cer previously approved these changes Sep 7, 2022
Copy link
Contributor

@ge65cer ge65cer left a comment

Choose a reason for hiding this comment

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

Tested locally, found no issues.

…ences

# Conflicts:
#	src/main/webapp/app/exam/manage/exam-status.component.ts
#	src/test/javascript/spec/component/exam/manage/student-exams/exam-status.component.spec.ts
#	src/test/javascript/spec/component/hestia/git-diff-report/full-git-diff-entry.component.spec.ts
#	src/test/javascript/spec/component/hestia/git-diff-report/full-git-diff-report.component.spec.ts
#	src/test/javascript/spec/component/programming-exercise/programming-exercise-detail.component.spec.ts
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 looks good. I did not find any other vialations in the code.

Copy link
Member

@julian-christl julian-christl left a comment

Choose a reason for hiding this comment

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

Reapprove code. New changes consistent

Copy link

@manuelmanso manuelmanso left a comment

Choose a reason for hiding this comment

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

Code looks good to me

Copy link
Contributor

@ge65cer ge65cer left a comment

Choose a reason for hiding this comment

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

Re-approve after merge

@krusche krusche merged commit 4ff0846 into develop Sep 13, 2022
@krusche krusche deleted the development/client/remove-isAtLeast-occurences branch September 13, 2022 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client Pull requests that update TypeScript code. (Added Automatically!) code quality documentation ready to merge refactoring tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants