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

[MichalOren]

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

[SejoB]

@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:

• pageNumber
• pageSize
• newsFlashId

api response should include:

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

[ziv17]

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]

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]

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]

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]

@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

[ziv17]

Hi @SejoB , @atalyaalon
I will try to sum it up:
Transition period, I.e. both old and new format are supported.

1. If both parameters pageNumber, nor newsFlashId are not provided, the current output format is used.
2. Parameters pageNumber and newsFlashId are disjoint, I.e. will not appear both in same request.
3. When one of pageNumber or newsFlashId appear, the new format (aka pagination) will be used for output.
4. pageNumber is not mandatory. It has a default of 0 (zero).
5. pageSize is not mandatory. It has a default of 50 (Do we want a different default?)
6. When pageNumber parameter is provided, the items of that page will be returned. The total number of items will be counted according to the filter parameters in the request.
7. When newsFlashId is provided:
   a) The offset of the item with the given id will be calculated. Output sorted ascending by Id, and using the provided filter fields.
   b) The items that will be returned correspond to using
   offset: given Id item offset – (total items /(2 * pageSize)) (of course, max between that value and 0)
   pageSize

After the transition period, the current out format, and the parameters offset and limit will not be supported.

[atalyaalon]

1-4 OK
5. I think that right now we're using in FE request 100 newsflashes every time (limit=100). Perhaps we can keep this as default pageSize? Or even increase it?
6. OK
7. Can you give an example for this formula explaining the logic?

@ziv17 all in all looks good. Please make sure performance stays the same with these changes (for example, when adding calculations like 7a).
@SejoB what do you think?

[atalyaalon]

Also, Regarding pageNumber, perhaps it conflicts with current implementation.
I think that pageSize is equivalent to limit.
Perhaps we can add a pagination param nextIdStartAfter that will indicate the first newsflash id in the next page, and will be used by FE to create the next page query.

This can be implemented differently of course, what ever seems right to both of you, as long as we're consistent.

@ziv17 @SejoB any thoughts?

[ziv17]

Hi,
the calculation of the offset of a record with a given newsFlashId is done as follows:
target_record_offset = SELECT count(*) FROM <table> WHERE <where part of other fields> AND id < newsFlashId ORDER BY id ASC
The offset for the query that retrieves the data is
offset_to_use: max(0, target_record_offset - floor(pageSize / 2))
and the query:
SELECT * FROM <table> WHERE <where part of other fields> OFFSET offset_to_user LIMIT pageSize ORDER BY id ASC

Regarding performance: This calculation requires two queries, so it will take more time. It is possible to do it in a single combined select statement (as I saw on the web) but I do not understand the syntax of that query, so I am reluctant to use it.

[ziv17]

Hi @atalyaalon , @SejoB ,
Also, Regarding pageNumber, perhaps it conflicts with current implementation. I think that pageSize is equivalent to limit. Perhaps we can add a pagination param nextIdStartAfter that will indicate the first newsflash id in the next page, and will be used by FE to create the next page query.

This can be implemented differently of course, what ever seems right to both of you, as long as we're consistent.

@ziv17 @SejoB any thoughts?

The pagination API is different from the current API. It works with pages rather than offset. I do not think there is a conflict since these are two different sets of parameters.
Regarding requests with id there are a few ways to go regarding what the user receives:

1. Place the requested record in the middle of the returned page. This will cause different pages than the pageNumber parameter would provide (i.e. page 1 will not necessarily start at offset 0). Subsequent pages will move by pageSize
2. First page will be like in option (1), but subsequent pages will use the pages that begin with offset 0. The first page-up or page-down may repeat or miss some records.
3. Always work with pages where pageNumber 1 starts at offset 0. Request with id will bring the page that holds the requested record, but it will not necessarily be placed in the middle.

Regarding implementation of API:

1. Option (1) above calls for using offset to specify the requested page, to avoid using two different ways to specify the current page when paging after id versus pageNumber. This will require FE to calculate the next offset.
2. Options (2) and (3) can always work with pageNumber for subsequent requests.

I suggest to go for option (3) above (not placing the requested record at the middle of the page). It is:

1. more consistent for the user. We can focus on that requested record so it is less important that it is not in the middle.
2. simpler implementation for both BE and FE.
3. requires less queries.

What do you think?

[ziv17]

Hi, @atalyaalon , @SejoB
pageNumber starts from 1, right?

[atalyaalon]

Hi @atalyaalon , @SejoB ,
Also, Regarding pageNumber, perhaps it conflicts with current implementation. I think that pageSize is equivalent to limit. Perhaps we can add a pagination param nextIdStartAfter that will indicate the first newsflash id in the next page, and will be used by FE to create the next page query.
This can be implemented differently of course, what ever seems right to both of you, as long as we're consistent.
@ziv17 @SejoB any thoughts?

The pagination API is different from the current API. It works with pages rather than offset. I do not think there is a conflict since these are two different sets of parameters. Regarding requests with id there are a few ways to go regarding what the user receives:

1. Place the requested record in the middle of the returned page. This will cause different pages than the pageNumber parameter would provide (i.e. page 1 will not necessarily start at offset 0). Subsequent pages will move by pageSize
2. First page will be like in option (1), but subsequent pages will use the pages that begin with offset 0. The first page-up or page-down may repeat or miss some records.
3. Always work with pages where pageNumber 1 starts at offset 0. Request with id will bring the page that holds the requested record, but it will not necessarily be placed in the middle.

Regarding implementation of API:

1. Option (1) above calls for using offset to specify the requested page, to avoid using two different ways to specify the current page when paging after id versus pageNumber. This will require FE to calculate the next offset.
2. Options (2) and (3) can always work with pageNumber for subsequent requests.

I suggest to go for option (3) above (not placing the requested record at the middle of the page). It is:

1. more consistent for the user. We can focus on that requested record so it is less important that it is not in the middle.
2. simpler implementation for both BE and FE.
3. requires less queries.

What do you think?

I'm good with option 3. @SejoB any thoughts?

[atalyaalon]

Hi, @atalyaalon , @SejoB pageNumber starts from 1, right?

Yes

[atalyaalon]

Hi, the calculation of the offset of a record with a given newsFlashId is done as follows: target_record_offset = SELECT count(*) FROM <table> WHERE <where part of other fields> AND id < newsFlashId ORDER BY id ASC The offset for the query that retrieves the data is offset_to_use: max(0, target_record_offset - floor(pageSize / 2)) and the query: SELECT * FROM <table> WHERE <where part of other fields> OFFSET offset_to_user LIMIT pageSize ORDER BY id ASC

Regarding performance: This calculation requires two queries, so it will take more time. It is possible to do it in a single combined select statement (as I saw on the web) but I do not understand the syntax of that query, so I am reluctant to use it.

OK.
Using 2 queries instead of 1 might still not be a performance issue.
However, If needed, I can help with query syntax if you want to use the single query instead of two.

[ziv17]

Hi @atalyaalon , @SejoB
What should be the response when pageNumber is larger than totalPages? Currently it just returns an empty list.

I have issued a PR

[atalyaalon]

@SejoB great work! (And of course @ziv17 great work as well!)

I merged into dev. All in all looks very good.

@SejoB There is only a small bug (using Chrome browser on mac)
For example, when using this link to this specific newsflash (id 222902), there is no possibility to scroll up. There is only scroll down, and only after scrolling down a page, we can scroll up again.

Attaching video. Can you please fix this on your time?

Screen.Recording.2024-11-28.at.19.28.42.mov

[atalyaalon]

@SejoB In addition to the bug above that needs to be fixed, there is a major bug when shifting between sources.
The page number keeps growing and we're getting older news flash every time - see the newsflashes dates.

See respective video:

Screen.Recording.2024-12-18.at.13.57.21.mov