Skip to content
This repository was archived by the owner on Apr 16, 2024. It is now read-only.

Repair notification settings #732

Merged
merged 6 commits into from
May 14, 2018

Conversation

kesselb
Copy link
Contributor

@kesselb kesselb commented May 9, 2018

Description:

Closes #731

Api endpoint getCourses returns ICourseDashboard instead ICourse since #654. As a result the students property is missing and notification settings are broken.

Improvements

  • Student can edit his notification settings
  • Migrate ICourse to ICourseDashbaord
  • Remove defunct isCourseTeacherOrAdmin and isMemberOfCourse
  • Use awesome SnackBarService for Snackbars 😁

Known Issues:

  • Some warnings in user-settings.component.html about missing or invalid attributes. Dont know howto fix them.

@kesselb kesselb changed the title Repair notification settings WIP: Repair notification settings May 9, 2018
@kesselb
Copy link
Contributor Author

kesselb commented May 9, 2018

Hey @AlperUygun, with #669 you implemented AfterContentInit for this service. I read the related issue and cant reproduce this issue after this pr even when AfterContentInit is no longer implemented. Could you verify this please? Thank you!

@coveralls
Copy link

coveralls commented May 9, 2018

Coverage Status

Coverage remained the same at 65.608% when pulling 56ec38d on bugfix/731-repair-notification-settings into e34aa05 on develop.

@kesselb kesselb changed the title WIP: Repair notification settings Repair notification settings May 10, 2018
Copy link
Collaborator

@PatrickSkowronek PatrickSkowronek left a comment

Choose a reason for hiding this comment

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

Nice Work

@PatrickSkowronek PatrickSkowronek merged commit a9a27e6 into develop May 14, 2018
@kesselb kesselb deleted the bugfix/731-repair-notification-settings branch May 14, 2018 11:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix notification settings
3 participants