Skip to content

Rename totalCounts with totalRecipientCount #335

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
Merged
merged 3 commits into from
Jul 30, 2025
Merged

Rename totalCounts with totalRecipientCount #335

merged 3 commits into from
Jul 30, 2025

Conversation

ylyangtw
Copy link
Contributor

@ylyangtw ylyangtw commented Jul 28, 2025 •
edited
Loading

Please test title I endpoints locally. You should see totalRecipientCount instead of totalCounts.

http://localhost:5000/pdl/titles/title-i/subtitles/subtitle-a/summary

http://localhost:5000/pdl/titles/title-i/subtitles/subtitle-d/summary

http://localhost:5000/pdl/titles/title-i/subtitles/subtitle-e/state-distribution

http://localhost:5000/pdl/titles/title-i/subtitles/subtitle-e/summary

@ylyangtw ylyangtw linked an issue Jul 28, 2025 that may be closed by this pull request
[TASK] Rename totalCounts with totalRecipientCount #334
Closed
pengyin-shan
pengyin-shan approved these changes Jul 28, 2025
View reviewed changes
Copy link
Contributor

@pengyin-shan pengyin-shan left a comment

Choose a reason for hiding this comment

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

Looks good and matchs the data-import PR. Approving it

@sandeep-ps
Copy link
Member

During review, I saw that in /pdl/titles/title-i/state-distribution, we are using totalRecipients. It will be good to replace that also to totalRecipientCount. Thanks.

@sandeep-ps
Copy link
Member

sandeep-ps commented Jul 28, 2025 •
edited
Loading

During review, I saw that in /pdl/titles/title-i/state-distribution, we are using totalRecipients. It will be good to replace that also to totalRecipientCount for consistency. Thanks.

Just to clarify, this comment was only about changes to Title-1 endpoints.

Title-II endpoints uses the same attribute name, but we shouldn't change that now. Thanks.

@ylyangtw
Copy link
Contributor Author

/pdl/titles/title-i/state-distribution

Good catch. Thanks Sandeep! I have addressed it

sandeep-ps
sandeep-ps approved these changes Jul 30, 2025
View reviewed changes
Copy link
Member

@sandeep-ps sandeep-ps left a comment

Choose a reason for hiding this comment

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

The changes look good. Approving. Thanks.

@ylyangtw ylyangtw merged commit 739713f into develop Jul 30, 2025
1 check passed
@ylyangtw ylyangtw deleted the 334-task-rename-totalcounts-with-totalrecipientcount branch July 30, 2025 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sandeep-ps sandeep-ps sandeep-ps approved these changes

@pengyin-shan pengyin-shan pengyin-shan approved these changes

@navarroc navarroc Awaiting requested review from navarroc

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[TASK] Rename totalCounts with totalRecipientCount
3 participants
@ylyangtw @sandeep-ps @pengyin-shan