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

Communication: Enable reactions if only messaging is enabled #7407

Conversation

lennart-keller
Copy link
Contributor

@lennart-keller lennart-keller commented Oct 17, 2023

Checklist

General

Server

  • Important: I implemented the changes with a very good performance and prevented too many (unnecessary) database calls.
  • I followed the coding and design guidelines.
  • I added multiple integration tests (Spring) related to the features (with a high test coverage).

Motivation and Context

Reactions only work if communication is enabled

Description

Reactions now also work, if only messaging is enabled.

Steps for Testing

Prerequisites:

  • 1 Student
  • 1 Instructor
  • 1 Course with only messaging enabled
  1. Check that you can react to messages and replies as both users and they get updated for the other user
  2. Enable communication, disable messaging
  3. Check that you can react to posts and replies as both users and reactions get updated for the other user

Review Progress

Performance Review

  • I (as a reviewer) confirm that the server changes (in particular related to database calls) are implemented with a very good performance

Code Review

  • Code Review 1
  • Code Review 2

Manual Tests

  • Test 1
  • Test 2

Test Coverage

Screenshots

No UI changes

@lennart-keller lennart-keller self-assigned this Oct 17, 2023
@lennart-keller lennart-keller requested a review from a team as a code owner October 17, 2023 14:20
@github-actions github-actions bot added the server Pull requests that update Java code. (Added Automatically!) label Oct 17, 2023
rstief
rstief previously approved these changes Oct 17, 2023
Copy link
Contributor

@rstief rstief 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 well.

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

Kroko-fant
Kroko-fant previously approved these changes Oct 17, 2023
Copy link
Contributor

@Kroko-fant Kroko-fant 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 ts looks good to me.
Code looks good to me

Strohgelaender
Strohgelaender previously approved these changes Oct 18, 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

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 + new tests lgtm

@lennart-keller
Copy link
Contributor Author

Is fixed with #7437

@lennart-keller lennart-keller deleted the bugfix/communication/reactions-if-only-messaging-is-enabled branch October 28, 2023 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
server Pull requests that update Java code. (Added Automatically!) tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants