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: Add additional tags to active user metrics #7240

Merged
merged 4 commits into from
Oct 3, 2023

Conversation

b-fein
Copy link
Contributor

@b-fein b-fein commented Sep 21, 2023

Note

Stacked on top of #7229

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).
  • I documented the Java code using JavaDoc style.

Motivation and Context

It might happen that the same course names and/or exam names are used multiple times in different semesters. This makes the tags non-unique and only the metrics for one of the courses/exams with the duplicate title are returned.

Description

  • Adds the courseId tag to both the course and exam metrics since we can be sure that those are unique.
  • Additionally refactors the code a bit to make it more readable.
  • Improves a database query to move the counting to the database to avoid loading the objects.

Steps for Testing

Same as in #7229, optional.

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

Test Coverage

Class/File Line Coverage Confirmation (assert/expect)
MetricsBean.java 87%, 100% for changed code

@github-actions github-actions bot added tests server Pull requests that update Java code. (Added Automatically!) labels Sep 21, 2023
@b-fein b-fein marked this pull request as ready for review September 21, 2023 19:08
@b-fein b-fein requested a review from a team as a code owner September 21, 2023 19:08
@b-fein b-fein requested a review from sleiss September 21, 2023 19:09
@b-fein b-fein force-pushed the metrics-add-course-id-tags branch 2 times, most recently from 32f8f61 to cf378bd Compare September 22, 2023 09:14
Base automatically changed from metrics-exclude-test-courses to develop September 22, 2023 14:06
@b-fein b-fein force-pushed the metrics-add-course-id-tags branch from cf378bd to 7500934 Compare September 22, 2023 22:21
Copy link
Contributor

@sleiss sleiss 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

@krusche krusche added this to the 6.5.2 milestone Sep 23, 2023
@krusche krusche modified the milestones: 6.5.2, 6.5.3 Sep 25, 2023
@krusche krusche modified the milestones: 6.5.3, 6.5.4 Sep 26, 2023
@b-fein b-fein requested a review from chrisknedl September 30, 2023 19:40
@krusche krusche merged commit 43ee4a5 into develop Oct 3, 2023
15 of 19 checks passed
@krusche krusche deleted the metrics-add-course-id-tags branch October 3, 2023 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ready to merge server Pull requests that update Java code. (Added Automatically!) small tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants