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

[#12048] Add integration tests for FeedbackResponseCommentsDb #12849

Merged
merged 62 commits into from
Mar 12, 2024
Merged

[#12048] Add integration tests for FeedbackResponseCommentsDb #12849

merged 62 commits into from
Mar 12, 2024

Conversation

xenosf
Copy link
Contributor

@xenosf xenosf commented Feb 26, 2024 •
edited
Loading

Part of #12048

Outline of Solution

@xenosf
Migrate SessionResultsData
e1955b6
@xenosf
Add default entities
8f0f835
@xenosf
Add helper methods to assist migrated logic
d40f2fb
@xenosf
Migrate buildCompleteGiverRecipientMap
da63d4a
@xenosf
Migrate checkSpecificAccessControl
cc8ff7a
@xenosf
Add default team instance for instructor
a046670
@xenosf
Migrate session results data logic
9b4945b
@xenosf
Use default team entity for instructor instead of const
fb5a808
@xenosf
Migrate non-db logic
cd64a4c
@xenosf
Merge branch 'v9-migration' into v9-migration-GetSessionResultsAction
a78bf65
@xenosf
Refactor Datastore and SQL action logic out to separate methods
97ae4b5
@xenosf
Fix checkstyle errors
51f738f
@xenosf
Migrate DB logic
6573955
@xenosf
Merge branch 'v9-migration' into v9-migration-GetSessionResultsAction
13191fe
@xenosf
Fix checkstyle errors
80d651e
@xenosf
Move default instructor team entity to const
7e960f7
@xenosf
Add test for SqlSessionResultsBundle
901307a
@xenosf
Fix SQL results bundle test
29900bd
@xenosf
Add IT for GetSessionResultsAction
ab20259
@xenosf
Fix action logic
06c6b61
@xenosf
Fix checkstyle errors
0ad3b56
@xenosf
Remove unused method parameters
a23545d
@xenosf
Merge branch 'v9-migration' into v9-migration-GetSessionResultsAction
1aa00a8
@xenosf
Fix persistence issues in test cases
56a9d93
@xenosf
Remove question getter for comment
a5de57d
@xenosf
Rename boolean methods to start with verb
6e62b70
@xenosf
Reword comment to clarify question ID
fa89559
@xenosf
Refactor getting question UUID from param value
273032b
@xenosf
Remove unneeded getters
8627c3f
@xenosf
Remove entities from Const
7f7b81b
@github-actions GitHub Actions
Copy link

Hi @xenosf, thank you for your interest in contributing to TEAMMATES!
However, your PR does not appear to follow our contribution guidelines:

  • Description must reference the issue number the PR is fixing, e.g. Fixes #<issue-number> (or Part of #<issue-number> if the PR does not address the issue fully)

Please address the above before we proceed to review your PR.

@xenosf xenosf force-pushed the v9-migration-add-tests-FeedbackResponseCommentsDb branch from 17c2659 to 8f3c2cb Compare February 26, 2024 17:28
@xenosf xenosf marked this pull request as ready for review February 26, 2024 17:29
@xenosf xenosf marked this pull request as draft February 26, 2024 17:34
@xenosf xenosf marked this pull request as ready for review February 26, 2024 17:35
@xenosf xenosf force-pushed the v9-migration-add-tests-FeedbackResponseCommentsDb branch from 8f3c2cb to fc290a1 Compare February 27, 2024 10:14
@domlimm domlimm mentioned this pull request Feb 27, 2024
Merged
cedricongjh
cedricongjh requested changes Feb 28, 2024
View reviewed changes
Copy link
Contributor

@cedricongjh cedricongjh left a comment

Choose a reason for hiding this comment

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

good test cases, LGTM! just 1 comment

src/it/java/teammates/it/storage/sqlapi/FeedbackResponseCommentsDbIT.java Outdated Show resolved Hide resolved
@cedricongjh cedricongjh added the s.Ongoing The PR is being worked on by the author(s) label Feb 28, 2024
@xenosf xenosf requested a review from cedricongjh March 12, 2024 17:03
cedricongjh
cedricongjh approved these changes Mar 12, 2024
View reviewed changes
Copy link
Contributor

@cedricongjh cedricongjh left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the work on this, especially creating the new data bundle

@cedricongjh cedricongjh added s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging and removed s.Ongoing The PR is being worked on by the author(s) labels Mar 12, 2024
@cedricongjh cedricongjh merged commit b802335 into TEAMMATES:master Mar 12, 2024
11 checks passed
@cedricongjh cedricongjh added this to the V9.0.0-beta.1 milestone Mar 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cedricongjh cedricongjh cedricongjh approved these changes

Assignees
No one assigned
Labels
s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging
Projects
None yet
Milestone
V9.0.0-beta.1
Development

Successfully merging this pull request may close these issues.
2 participants
@xenosf @cedricongjh