-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Update GetUsageStatisticsAction to include SQL entities #12084
Update GetUsageStatisticsAction to include SQL entities #12084
Conversation
@daongochieu2810, we need unit tests for this and the db layer. Let's not relegate testing to an afterthought, sets a bad precedent for future PRs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need some tests for this. Some simple integration tests (basically just the success case + any cases that cannot be tested via unit tests), defined in the it package and unit tests for each component (action, db and non-trivial logic methods).
src/main/java/teammates/ui/output/UsageStatisticsRangeData.java
Outdated
Show resolved
Hide resolved
int numResponses = 0; //feedbackResponsesLogic.getNumFeedbackResponsesByTimeRange(startTime, endTime); | ||
int numCourses = 0; //coursesLogic.getNumCoursesByTimeRange(startTime, endTime); | ||
int numStudents = 0; //studentsLogic.getNumStudentsByTimeRange(startTime, endTime); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that we will need to support the old methods as well. Course related entities will not be migrated immediately, so there will be a period of time we have to fetch statistics for both new and old entities.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@samuelfangjw I will make sure to do this in CalculateUsageStatisticsAction
in the next PR
src/main/java/teammates/sqllogic/core/UsageStatisticsLogic.java
Outdated
Show resolved
Hide resolved
src/main/java/teammates/sqllogic/core/UsageStatisticsLogic.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Requesting a minor change to keep things clean, LGTM otherwise!
* [#12048] Set up github action workflows * [#12048] v9: Skeleton implementation (#12056) * [#12048] Add isMigrated flag to course (#12063) * [#12048] Add is migrated flag to datastore account (#12070) * Temporarily disable liquibase migrations * [#12048] Create Notification Entity for PostgreSQL migration (#12061) * [#12048] Create notification DB layer for v9 migration (#12075) * [#12048] Add UsageStatistics entity and db (#12076) * Migrate GetCourseAction.java * Migrate DeleteCourseAction.java and relevant logic functions * Migrate BinCourseAction.java and its related logic functions * Update checkSpecificAccessControl functions in BinCourseAction and DeleteCourseAction classes * Migrate RestoreCourseAction and its related logic functions * Migrate UpdateCourseAction with its related logic functions * [#12048] Add Account Entity (#12087) * [#12048] Create SQL logic for CreateNotificationAction and add relevant tests for v9 migration (#12077) * [#12048] Create Student, Instructor and User Entities for PostgreSQL Migration (#12071) * [#12048] V9: Cleanup and refactor (#12090) * Edit GetCourseAction and refactor out the old datastore code * [#12048] Remove redundant InstructorRole Enum (#12091) * Fix compilation error * Update check for database to fetch from * Add unit tests for CoursesDb * [#12048] Update GetUsageStatisticsAction to include SQL entities (#12084) * Add CoursesLogicTest class * Disable failing tests * Fix compilation error * Fix Checkstyle errors * Merge branch * Change flow for updating courses. * Update updateCourse JavaDoc comment. * Update CreateCourseAction and related methods * Update GetCourseAction. * Update UpdateCourseAction * Update BinCourseAction and RestoreCourseAction * Update DeleteCourseAction * Migrate GetCourseSectionNamesAction and related methods. * Add Unit tests for Logic layer of Course. * Fix Checkstyle errors * Add unit test for GetCourseAction's execute function * Add verify for CoursesDb unit tests and use assertNull and assertNotNull * Move fetching of course to logic layer. * Fix Checkstyle errors. * Move canCreateCourse logic to logic layer. * Change *CourseAction classes to use isCourseMigrated * Fix CoursesLogic's initLogicDependencies method call * Add unit tests for GetCourseAction. * Remove commented out method. * Add minimal unit tests for BinCourseAction, DeleteCourseAction and RestoreCourseAction. * Add minimal unit tests for GetCourseSectionAction and UpdateCourseAction. * Remove unused EntityType parameter. * Add minimal unit tests for CreateCourseAction. * Fix Checkstyle errors. * Ignore all old datastore test cases for *CourseAction classes. * Fix 'text' type to 'test'. * Change binCourseToRecycleBin to return the binned course. * Update moveCourseToRecycleBin test. * Update test name. --------- Co-authored-by: Samuel Fang <[email protected]> Co-authored-by: dao ngoc hieu <[email protected]> Co-authored-by: Samuel Fang <[email protected]> Co-authored-by: wuqirui <[email protected]> Co-authored-by: Dominic Lim <[email protected]>
* [#12048] Set up github action workflows * [#12048] v9: Skeleton implementation (#12056) * [#12048] Add isMigrated flag to course (#12063) * [#12048] Add is migrated flag to datastore account (#12070) * Temporarily disable liquibase migrations * [#12048] Create Notification Entity for PostgreSQL migration (#12061) * [#12048] Create notification DB layer for v9 migration (#12075) * [#12048] Add UsageStatistics entity and db (#12076) * Migrate GetCourseAction.java * Migrate DeleteCourseAction.java and relevant logic functions * Migrate BinCourseAction.java and its related logic functions * Update checkSpecificAccessControl functions in BinCourseAction and DeleteCourseAction classes * Migrate RestoreCourseAction and its related logic functions * Migrate UpdateCourseAction with its related logic functions * [#12048] Add Account Entity (#12087) * [#12048] Create SQL logic for CreateNotificationAction and add relevant tests for v9 migration (#12077) * [#12048] Create Student, Instructor and User Entities for PostgreSQL Migration (#12071) * [#12048] V9: Cleanup and refactor (#12090) * Edit GetCourseAction and refactor out the old datastore code * [#12048] Remove redundant InstructorRole Enum (#12091) * Fix compilation error * Update check for database to fetch from * Add unit tests for CoursesDb * [#12048] Update GetUsageStatisticsAction to include SQL entities (#12084) * Add CoursesLogicTest class * Disable failing tests * Fix compilation error * Fix Checkstyle errors * Merge branch * Change flow for updating courses. * Update updateCourse JavaDoc comment. * Update CreateCourseAction and related methods * Update GetCourseAction. * Update UpdateCourseAction * Update BinCourseAction and RestoreCourseAction * Update DeleteCourseAction * Migrate GetCourseSectionNamesAction and related methods. * Add Unit tests for Logic layer of Course. * Fix Checkstyle errors * Add unit test for GetCourseAction's execute function * Add verify for CoursesDb unit tests and use assertNull and assertNotNull * Move fetching of course to logic layer. * Fix Checkstyle errors. * Move canCreateCourse logic to logic layer. * Change *CourseAction classes to use isCourseMigrated * Fix CoursesLogic's initLogicDependencies method call * Add unit tests for GetCourseAction. * Remove commented out method. * Add minimal unit tests for BinCourseAction, DeleteCourseAction and RestoreCourseAction. * Add minimal unit tests for GetCourseSectionAction and UpdateCourseAction. * Remove unused EntityType parameter. * Add minimal unit tests for CreateCourseAction. * Fix Checkstyle errors. * Ignore all old datastore test cases for *CourseAction classes. * Fix 'text' type to 'test'. * Change binCourseToRecycleBin to return the binned course. * Update moveCourseToRecycleBin test. * Update test name. --------- Co-authored-by: Samuel Fang <[email protected]> Co-authored-by: dao ngoc hieu <[email protected]> Co-authored-by: Samuel Fang <[email protected]> Co-authored-by: wuqirui <[email protected]> Co-authored-by: Dominic Lim <[email protected]>
* [#12048] Set up github action workflows * [#12048] v9: Skeleton implementation (#12056) * [#12048] Add isMigrated flag to course (#12063) * [#12048] Add is migrated flag to datastore account (#12070) * Temporarily disable liquibase migrations * [#12048] Create Notification Entity for PostgreSQL migration (#12061) * [#12048] Create notification DB layer for v9 migration (#12075) * [#12048] Add UsageStatistics entity and db (#12076) * Migrate GetCourseAction.java * Migrate DeleteCourseAction.java and relevant logic functions * Migrate BinCourseAction.java and its related logic functions * Update checkSpecificAccessControl functions in BinCourseAction and DeleteCourseAction classes * Migrate RestoreCourseAction and its related logic functions * Migrate UpdateCourseAction with its related logic functions * [#12048] Add Account Entity (#12087) * [#12048] Create SQL logic for CreateNotificationAction and add relevant tests for v9 migration (#12077) * [#12048] Create Student, Instructor and User Entities for PostgreSQL Migration (#12071) * [#12048] V9: Cleanup and refactor (#12090) * Edit GetCourseAction and refactor out the old datastore code * [#12048] Remove redundant InstructorRole Enum (#12091) * Fix compilation error * Update check for database to fetch from * Add unit tests for CoursesDb * [#12048] Update GetUsageStatisticsAction to include SQL entities (#12084) * Add CoursesLogicTest class * Disable failing tests * Fix compilation error * Fix Checkstyle errors * Merge branch * Change flow for updating courses. * Update updateCourse JavaDoc comment. * Update CreateCourseAction and related methods * Update GetCourseAction. * Update UpdateCourseAction * Update BinCourseAction and RestoreCourseAction * Update DeleteCourseAction * Migrate GetCourseSectionNamesAction and related methods. * Add Unit tests for Logic layer of Course. * Fix Checkstyle errors * Add unit test for GetCourseAction's execute function * Add verify for CoursesDb unit tests and use assertNull and assertNotNull * Move fetching of course to logic layer. * Fix Checkstyle errors. * Move canCreateCourse logic to logic layer. * Change *CourseAction classes to use isCourseMigrated * Fix CoursesLogic's initLogicDependencies method call * Add unit tests for GetCourseAction. * Remove commented out method. * Add minimal unit tests for BinCourseAction, DeleteCourseAction and RestoreCourseAction. * Add minimal unit tests for GetCourseSectionAction and UpdateCourseAction. * Remove unused EntityType parameter. * Add minimal unit tests for CreateCourseAction. * Fix Checkstyle errors. * Ignore all old datastore test cases for *CourseAction classes. * Fix 'text' type to 'test'. * Change binCourseToRecycleBin to return the binned course. * Update moveCourseToRecycleBin test. * Update test name. --------- Co-authored-by: Samuel Fang <[email protected]> Co-authored-by: dao ngoc hieu <[email protected]> Co-authored-by: Samuel Fang <[email protected]> Co-authored-by: wuqirui <[email protected]> Co-authored-by: Dominic Lim <[email protected]>
Part of #12048
Outline of Solution
UsageStatisticsRangeData
to support mapping from the sql entities