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

Moves Async entity Implementation to Core #2732

Merged
merged 8 commits into from
Apr 10, 2024
Merged

Conversation

shubham1g5
Copy link
Contributor

@shubham1g5 shubham1g5 commented Nov 29, 2023

Summary

https://dimagi-dev.atlassian.net/browse/USH-3790

Moves Async Entity to core in order to be able to implement Cache and Indexing feature on Web Apps.

Feature Flag

Cache and Index

Safety Assurance

  • If the PR is high risk, "High Risk" label is set
  • I have confidence that this PR will not introduce a regression for the reasons below
  • Do we need to enhance manual QA test coverage ? If yes, "QA Note" label is set correctly

Automated test coverage

No substantial coverage

Safety story

  • Behind feature flag
  • Refactor is straightforward without making any sort of functional changes
  • Will go through QA

QA Note: Test Cache and Index functionality.

review commit by commit

cross-request: dimagi/commcare-core#1387

@shubham1g5
Copy link
Contributor Author

bumping this for a review.

Copy link
Contributor

@avazirna avazirna left a comment

Choose a reason for hiding this comment

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

@shubham1g5 looks good, I just left a minor comment.

return detailId + "_" + mFieldId;
}

private static String buildValidKeys(Vector<Integer> sortKeys, DetailField[] fields) {
Copy link
Contributor

Choose a reason for hiding this comment

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

buildValidKeys, buildKeyNameWhereClause and populateEntitySet are private static methods called by non-static methods, do they need to be static?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I prefer to make methods private static when they can be to signify that none of these methods changes the member variables of the class.

@shubham1g5
Copy link
Contributor Author

@damagatchi retest this please

@shubham1g5 shubham1g5 merged commit 4637f56 into master Apr 10, 2024
4 of 6 checks passed
@shubham1g5 shubham1g5 deleted the asyncEntityRefactor branch April 10, 2024 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants