-
Notifications
You must be signed in to change notification settings - Fork 554
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
filter blog by topics on render article route #25118
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…c/filter-capi-data-auto-filters
…er-capi-data-auto-filters
…c/filter-capi-data-auto-filters
…capi-data-auto-filters
…capi-data-auto-filters
Base automatically changed from
marji/integrate-mentions-in-liveblog-controller
to
main
June 20, 2022 09:57
marjisound
force-pushed
the
jc/filter-capi-data-auto-filters
branch
from
June 20, 2022 14:29
01ff13b
to
2666317
Compare
marjisound
changed the title
Jc/filter capi data auto filters
filter blog by topics on render article route
Jun 21, 2022
marjisound
force-pushed
the
jc/filter-capi-data-auto-filters
branch
from
June 21, 2022 08:17
016f495
to
d53c18b
Compare
marjisound
force-pushed
the
jc/filter-capi-data-auto-filters
branch
from
June 21, 2022 13:51
0256876
to
bac6189
Compare
Hi all, I know there's been discussion about naming of different models or classes. There's an upcoming PR that we're currently working on which is supposed to factor all these name changes #25133 |
alinaboghiu
approved these changes
Jun 22, 2022
This PR was fully tested in code so I'll merge it to unblock the rest of the work. But if you have any feedback after the merge, please add them here and I'll make sure to address them in the next PR :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this change?
This PR adds logic for filtering liveblog on render article route. It'd add implementation for pagination when top mention result is provided.
TopicsLiveBlog
BlockRange for when first page is requested with topicsfirstPage
logic to have correct pagination when top mentions result is providedfindPageWithBlock
to have correct pagination when top mentions result is providedfirstPage
implementation and extracts the details into separate private functionsapplyFilters
implementation for when top mentions is providedfirstPage
&findPageWithBlock
for when top mentions is providedDCRFake
in order to enable testing of the the data passed to DCRrenderArticle
endpointpaired with @joecowton1
Does this change need to be reproduced in dotcom-rendering ?
Screenshots
What is the value of this and can you measure success?
Checklist
Does this affect other platforms?
Does this affect GLabs Paid Content Pages? Should it have support for Paid Content?
Does this change break ad-free?
Does this change update the version of CAPI we're using?
Accessibility test checklist
Tested