data-for-change / anyway-newsflash-infographics

Development branch deployed automatically to website:
https://anyway-infographics-staging.web.app/
MIT License
12 stars 23 forks source link

[Bug]: Coordinate the infographics main screen with the newsflash list #1051

Open MichalOren opened 6 months ago

MichalOren commented 6 months ago

Describe the bug There is a mismatch between the infographics screen and the news flash list on the right. The problem exists in several flows, therefore it is described in general terms.

To Reproduce https://anyway-infographics-staging.web.app/newsflash/184863

Expected behavior

image

SejoB commented 6 months ago

@atalyaalon

Today api haven't indication about pagination. Api receive only offset and limit.

To achieve this feature we need to add full pagination control to api request.

As well in this case we need bi direction pagination.

api params to add:

api response should include:

data: [] pagination :{ pageNumber: number; pageSize: number; totalPages: number; totalRecords: number; }

ziv17 commented 1 week ago

Hi @SejoB , I will be implementing this on the back-end. I have a question: What should be the behavior when newsFlashId parameter that you mentioned is provided? Is it mutually disjoint from pageNumber and pageSize? Thanks, Ziv

ziv17 commented 1 week ago

Hi @SejoB , For the integration, I can output the new format only when pageNumber is provided, and only on second step to remove the current output format completely. What do you think?

atalyaalon commented 6 days ago

Hi @SejoB , I will be implementing this on the back-end. I have a question: What should be the behavior when newsFlashId parameter that you mentioned is provided? Is it mutually disjoint from pageNumber and pageSize? Thanks, Ziv

@ziv17 from my understanding, when newsFlashId parameter is provided together with and limit (and optional pageNumber and pageSize), ideal behavior is sending news flashes that are chronologically before and after this newsflash (say if limit is 100, so we'll get 50 newsflashes before and 49 newsflashes after specific newsflash - or less if there are less). Meaning that to some extent newsFlashId sets the offset in this case, and seems like these two parameters cannot co-exist.

@SejoB @ziv17 What do you think?

atalyaalon commented 6 days ago

Hi @SejoB , For the integration, I can output the new format only when pageNumber is provided, and only on second step to remove the current output format completely. What do you think?

Sounds good to me

atalyaalon commented 6 days ago

@ziv17 Just to make sure we're aligned with current functionality (that will either be changed or deprecated if we'll use newsFlashId as a parameter): Today when fetching specific news flash id, it's not a parameter but a part of the main URL for example: https://www.anyway.co.il/api/news-flash/214463

And there is no option to fetch additional newsflashes using limit. We'll get the same response when using this URL as well: https://www.anyway.co.il/api/news-flash/214463?limit=100