Skip to content

Conversation

@ndegwamartin
Copy link

@ndegwamartin ndegwamartin commented Jul 22, 2023

@ndegwamartin ndegwamartin force-pushed the multiple-practitioner-details branch from 7146c4b to daded4c Compare July 22, 2023 12:37
@ndegwamartin ndegwamartin force-pushed the multiple-practitioner-details branch from bce891a to ddd8d9e Compare July 28, 2023 14:38
import org.smartregister.utils.Constants;
import org.springframework.lang.Nullable;

public class OpenSRPHelper {
Copy link
Member

Choose a reason for hiding this comment

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

can we rename this class with something more semantic? Like RelationshipManagementHelper

Copy link
Author

Choose a reason for hiding this comment

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

The idea was that it would be generic to contain other helper methods for extended functionality. Right now it only supports the multi-practitioner details endpoint so maybe for now we can re-name it to something that alludes to that.

Copy link
Author

Choose a reason for hiding this comment

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

Renamed it from OpenSRPHelper to PractitionerDetailsEndpointHelper

@ndegwamartin ndegwamartin force-pushed the multiple-practitioner-details branch from ee5b31a to cb0f739 Compare August 1, 2023 07:01
@pld
Copy link
Member

pld commented Aug 1, 2023 via email

Copy link
Member

Choose a reason for hiding this comment

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

cool, so I know this is an existing class, but think it should be renamed to simply

SyncAccessDecision

I don't see this as OpenSRP specific

Copy link
Author

Choose a reason for hiding this comment

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

Cool, I can rename this on the PR cc @rehammuzzamil and @dubdabasoduba

@ndegwamartin ndegwamartin removed the request for review from dubdabasoduba August 2, 2023 14:30
@ndegwamartin ndegwamartin merged commit 3355855 into main Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Server Side] Adding multiple practitioner details endpoint

3 participants