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.

Sign up for GitHub

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

Jump to bottom

Create Notification Entity for PostgreSQL migration #12061

Merged
merged 7 commits into from
Feb 10, 2023
Merged

Create Notification Entity for PostgreSQL migration #12061

merged 7 commits into from
Feb 10, 2023

Conversation

hhdqirui
Copy link
Member

@hhdqirui hhdqirui commented Feb 8, 2023

Part of #12048

Create Notification entity to be used for PostgreSQL migration.

@hhdqirui hhdqirui added the s.ToReview The PR is waiting for review(s) label Feb 8, 2023
samuelfangjw
samuelfangjw reviewed Feb 10, 2023
View reviewed changes
src/main/java/teammates/storage/sqlentity/Notification.java Outdated
Comment on lines 33 to 34
@Id
private String notificationId;
Copy link
Member

Choose a reason for hiding this comment

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

@hhdqirui Should we autogenerate this field instead @GeneratedValue(strategy = GenerationType.UUID). Right now seems like the calling function (that initialises the builder obj) is the one that is responsible for generation of UUID, which may not be what we want.

Copy link
Member

Choose a reason for hiding this comment

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

Also we should consider using the UUID type and only typecasting when necessary, might be easier to work with.

Copy link
Member Author

@hhdqirui hhdqirui Feb 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I agree that using UUID would be better and @GeneratedValue(strategy = GenerationType.UUID).

Right now seems like the calling function (that initialises the builder obj) is the one that is responsible for generation of UUID, which may not be what we want.

This is actually what we have right now as defined in CreateNotificationAction. But of course, changing it does not really have any impact. Will update.

@samuelfangjw samuelfangjw merged commit 9198839 into TEAMMATES:v9-migration Feb 10, 2023
domlimm pushed a commit to domlimm/teammates that referenced this pull request Feb 22, 2023
@hhdqirui @domlimm
[TEAMMATES#12048] Create Notification Entity for PostgreSQL migration (…
997bf6d 
…TEAMMATES#12061)
samuelfangjw pushed a commit that referenced this pull request Feb 26, 2023
@hhdqirui @samuelfangjw
[#12048] Create Notification Entity for PostgreSQL migration (#12061)
0f9c53a
hhdqirui added a commit that referenced this pull request Mar 20, 2023
@EuniceSim142 @samuelfangjw
@daongochieu2810 @hhdqirui @domlimm
[#12048] Migrate Course Action classes (#12092)
6c7c602 
* [#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 <samuelfangjw@gmail.com>
Co-authored-by: dao ngoc hieu <53283766+daongochieu2810@users.noreply.github.com>
Co-authored-by: Samuel Fang <60355570+samuelfangjw@users.noreply.github.com>
Co-authored-by: wuqirui <53338059+hhdqirui@users.noreply.github.com>
Co-authored-by: Dominic Lim <46486515+domlimm@users.noreply.github.com>
samuelfangjw added a commit that referenced this pull request Mar 20, 2023
@EuniceSim142 @samuelfangjw
@daongochieu2810 @hhdqirui @domlimm
[#12048] Migrate Course Action classes (#12092)
839c642 
* [#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 <samuelfangjw@gmail.com>
Co-authored-by: dao ngoc hieu <53283766+daongochieu2810@users.noreply.github.com>
Co-authored-by: Samuel Fang <60355570+samuelfangjw@users.noreply.github.com>
Co-authored-by: wuqirui <53338059+hhdqirui@users.noreply.github.com>
Co-authored-by: Dominic Lim <46486515+domlimm@users.noreply.github.com>
samuelfangjw pushed a commit that referenced this pull request Apr 8, 2023
@hhdqirui @samuelfangjw
[#12048] Create Notification Entity for PostgreSQL migration (#12061)
a510f0e
samuelfangjw added a commit that referenced this pull request Apr 8, 2023
@EuniceSim142 @samuelfangjw
@daongochieu2810 @hhdqirui @domlimm
[#12048] Migrate Course Action classes (#12092)
6c6a82f 
* [#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 <samuelfangjw@gmail.com>
Co-authored-by: dao ngoc hieu <53283766+daongochieu2810@users.noreply.github.com>
Co-authored-by: Samuel Fang <60355570+samuelfangjw@users.noreply.github.com>
Co-authored-by: wuqirui <53338059+hhdqirui@users.noreply.github.com>
Co-authored-by: Dominic Lim <46486515+domlimm@users.noreply.github.com>
@wkurniawan07 wkurniawan07 added s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging c.Task Other non-user-facing works, e.g. refactoring, adding tests and removed s.ToReview The PR is waiting for review(s) labels Jan 21, 2024
@wkurniawan07 wkurniawan07 added this to the V9.0.0-beta.0 milestone Jan 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jianhandev jianhandev jianhandev left review comments

@samuelfangjw samuelfangjw samuelfangjw approved these changes

Assignees
No one assigned
Labels
c.Task Other non-user-facing works, e.g. refactoring, adding tests s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging
Projects
None yet
Milestone
V9.0.0-beta.0
Development

Successfully merging this pull request may close these issues.
4 participants
@hhdqirui @jianhandev @samuelfangjw @wkurniawan07