Hide "Show all editions" link in edition table when appropriate

[Spaarsh]

The issue#9946 was occurring due to "?mode=ebooks" being default at some point in time earlier. Since that is not the case anymore, according to the openlibrary/openlibrary/templates/type/edition/view.html, the editions_limit was being assigned the value 10 always. This prevented the default value of editions_limit to be None (which is then used in openlibrary/openlibrary/templates/type/work/editions_database.html).

The change includes the 'None' to be one of the acceptable states, restoring the originally intended logic.

Closes #9946

fix

Technical

The implementations simply adds None into the set of acceptable states of the query parameter 'mode'.

Testing

1. Go to the homepage and search for any book.
2. Click upon any book in the results.
3. Scroll down and look at the editions table.
4. If the "Showing X featured editions. View all Y editions?" text doesn't show anymore for cases where X is equal to Y (Img. 1), the issue has been fixed. And otherwise, it should show the aforementioned text (Img. 2).

Screenshot

Img. 1(when X==Y)

Img. 2 (when X!=Y)

Stakeholders

@SivanC @jimchamp @cdrini

[cdrini]

@SivanC would you be able to give this one a code review and QA?

[SivanC]

@SivanC would you be able to give this one a code review and QA?

@cdrini Yes! Sorry should've realized that's my jurisdiction 😅

[cdrini]

Sweet ty! Haha, I needed you to leave a comment, otherwise GitHub won't let me assign you 😁

[SivanC]

@cdrini @Spaarsh Looks good to me! No text in ?mode=foo/?mode=all or by default, showed in ?mode=ebooks when there was 1/3 ebook editions.

[Spaarsh]

@SivanC waiting for it to be merged!

[SivanC]

@Spaarsh Thanks for reaching out! Unfortunately I'm not in a position to be able to merge your PR, but I will contact those who can again and see that it gets merged soon.

[jimchamp]

Thanks for your patience, @Spaarsh!

When I deployed this to our testing environment today, I noticed that the featured edition counts and the table entries for some book pages were different in production.

Work OL77764W in production environment

Work OL77764W in testing environment

If mode exists, then we expect it to be either all or ebooks. Because of this, these changes effectively set limit to None every time the page is loaded (the exception being when mode is manually set to an unexpected value, like baz).

Limiting the number of editions that we fetch for a book page is very important, especially when the work has several hundred editions. Page rendering is blocked by the get_sorted_editions call, and waiting for this method to return can take tens of seconds in the worst case.

Can you roll this change back, and instead update this condition in editions_datatable.html? I think that something like this would work:
(editions_limit and len(editions) < editions_limit) or (len(editions) < work.edition_count)

[Spaarsh]

Sorry for the delay! The change suggested by you didn't work, so made one small change, I flipped the less than sign to a greater sign:

(editions_limit and len(editions) > editions_limit) or (len(editions) < work.edition_count)

Explanation:

The editions_limit is set to None if the mode is ebooks or all, otherwise, it is set to be 10 as per this line in view.html.

Mode not set or unexpected mode selected

So, when there is no mode, 10 is the editions_limit, hence the first part of the statement is:
10 and len(editions)>10

Now, if the editions themselves are 8 (as in the case of this work), there is no need to display the "view all editions" part. And accordingly, the statement len(editions) > editions_limit resolves to False resulting in the first to be False.

Moving on to the next part len(edtions) < work.edition_count, when no mode is selected, the len(editions) is equal to the work.edition_count. So the condition return False and accordingly, we get the entire condition to be False.

False or False which resolves to False. Thus, the "view all editions" is not displayed.

Mode set to 'foo' for this work

No mode set for this work

No mode set for this work

Mode set to "all" or "ebooks"

When the mode is selected to be ebooks or all, the editions_limit is set to None so the first part resolves to a False.

Mode set to "all"

Moving to the next part, if the mode==all then len(editions) is equal work.edition_count. Hence the entire condition returns a False. So the "view all editions" shall not be displayed.

All mode set for this work

Mode set to "ebooks"

While for the ebooks mode, the len(editions) is less than work.edition_count. Thus the second part of the condition returns True and hence, the entire condition is True so the "view all editions" is displayed.

ebooks mode set for this work

[jimchamp]

Thanks @Spaarsh! Lgtm.