Next issue / Previous issue are not always the good issue

[BRUCELLA2]

Describe the bug
When we go from issue to issue with the "Previous Issue" button or "Next Issue" button, the next (or the previous) issue is not good. If my issue is 1 the next issue is 10 (see capture)

To Reproduce
Steps to reproduce the behavior:

1. Connect with admin account
2. Go to Comics section
3. Right clic and open the first issue Comic of a serie with more than 10 issues
4. Clic on Next issue button => we are on the issue 10

Expected behavior
if i'm on issue 1 the next issue is the issue 2

Screenshots

Desktop (please complete the following information):

ComiXed version : 0.6.0-0.rc7
OS: Windows 10 and Linux Mint 19.3 Tricia
Linux Browser name : Brave Version 1.9.76 Chromium: 81.0.4044.138 (Official Build) (64-bit) and firefox 77.0.1 (64 bits)
Windows Browser name : Brave Version 1.9.76 Chromium: 81.0.4044.138 (Official Build) (64-bit) and Chrome 83.0.4103.97 (Official Build) (64-bit)

ComiXed Build Detail
Source Branch c6993d1
Build Hostname fv-az129
Built On Jun 7, 2020, 12:20:01 AM
Version 0.6.0-0.rc7
Commit Hash c6993d1
Committer Name Darryl L. Pierce (mcpierce@gmail.com)
Time Of Last Commit Jun 7, 2020, 12:13:51 AM
Commit Message Added tags for v0.6.0-0.rc7 [#303]
Built From Uncommitted Code? true
Remote Origin https://github.com/comixed/comixed

Additional context
Add any other context about the problem here.

[BRUCELLA2]

A look at the code shows that the issue is a string and that strings are compared to get the next or previous issues.
I think to fix quickly the problem, we can modify the sql query (in ComicRepository) like this :
@query("SELECT c FROM Comic c WHERE c.series = :series and c.volume = :volume AND CONVERT(c.issueNumber, NUMERIC) < CONVERT(:issueNumber, NUMERIC) ORDER BY CONVERT(c.issueNumber, NUMERIC) DESC")

[mcpierce]

@BRUCELLA2 That's a good suggestion. Though the problem we'd then hit is that issue numbers aren't always numeric: I have comics in my library that have alphanumeric issue numbers. What we could do is use the Comic.sortableIssueNumber field, possibly. Or else use the cover date instead of the issue number since one would assume the cover date is more precisely (remember DC's New 52 put out issue #0 after issue #12?).

[BRUCELLA2]

I suspected there was a good reason for things to be implemented the way they are. :)
The cover date seems to meet the different constraints (the # that don't follow each other, the non-numerical #...)

[mcpierce]

@BRUCELLA2 Agreed. I've got a solution for you to check and will be pushing it shortly after I test it a bit. Thanks for finding this!

[mcpierce]

@BRUCELLA2 The fix for this is now committed on develop and I'm pulling it into the release branch for rc8. Please close this when you have a chance to verify it's fixed.

[BRUCELLA2]

It doesn't seem to be working out as expected.

Ex :

• I open Re-implement the multi-comic scraping workflow #1 (cover date = Nov 26, 2011). I clic on Next issue and i have FreeBSD support #10 (cover date = Aug 26, 2012) ... the Duplicates pages are always returning blocked=false even when the page hash is blocked #2 was expected with cover date Dec 31, 2011) ... Maybe it's an "order by" problem on the request. (with : "ORDER BY c.coverDate, c.issueNumber ASC" seems to work).

• I open After performing any action on selected comics, they should be removed from the selection state. #11 (cover date = Sep 26, 2012). i clic on Previous issue and i have After importing comics, not all are being returned while being processed #9 (cover date = Jul 26, 2012) ... the FreeBSD support #10 was expected with code date Aug 26, 2012... with all # > 9 the previous issue button led to After importing comics, not all are being returned while being processed #9 ... again maybe it's an "order by" problem on the request. (with : "ORDER BY c.coverDate, c.issueNumber DESC" seems to work).

• Last thing the specific case of #0. #0 is after Record the number of files in an archive during the import process #12 and before Saved sort order in Series page isn't restored when the page loads #13 ... this don't work. Don't you think there is a little mistake in if condition line 248 and 272. Shouldn't the "&& (candidate.getSortableIssueNumber().compareTo(result.getSortableIssueNumber()) ... " be " || ..." condition ?

[mcpierce]

The logic in the code is that next issue is determined first by the cover date and then by the issue number; i.e., i've got multiple comics that have the same cover date but different issue numbers in the 2016 volume of Action Comics.

The check needs to be a a logical AND because, otherwise, the issue navigation gets into a loop. If issue X and Y have the same cover date then they will always pass the cover date comparison and will get into a loop of being each other's next and previous issue. So the next issue needs to always have a cover date that's greater than or equal to the current issue and also have a greater issue number, and the previous a lower issue number.

I had considered the problem with an issue 0. Unfortunately, it falls into a very hard to handle edge case that a certain other comic library didn't solve either 😄. That said, I supposed a further change would be to do a cover date check and, if they're different return the next/previous issue. Otherwise (since they'll have the same cover date) return the next higher or lower issue number. I'll add that enhancement and we can try again.

[mcpierce]

@BRUCELLA2 I split the logic into two pieces: first it checks if the cover dates are different and returns, otherwise it compares the issue numbers and returns, otherwise it moves on. I'm able to scroll up and down each series I've tested locally. The behavior you're seeing is what I saw prior to the changes on this issue, so check the build details and make sure you're running against the development build that includes the fix.

I'm checking in a new set of changes now.

[bareheiny]

0 and 1/2 issues will cause problems if the sort is based on cover date.

DCs Countdown will be a problem if sorted numerically.

Gen 13 has three 13 issues - 13a, 13b and 13c - haven’t bothered to check the cover date for them yet.

How does the sort order field fit into this?

[mcpierce]

@bareheiny The sorting treats the issue numbers as strings rather than numbers so the sorting will be alphanumeric. But, that said, it should only fallback to the issue number when the cover date are exactly the same.

I think it's probably exceedingly unlikely that we'll get into a situation where we get odd issue numbers for multiple comics in the same series and volume published on the same date.

[mcpierce]

@BRUCELLA2 The changes should be included in the next incremental build. Please make sure you're running that when verifying.

[BRUCELLA2]

There is a problem somewhere.

ComiXed Build Details
Source Branch | develop
Build Hostname | fred-MS-7821
Built On | Jun 11, 2020, 12:34:46 AM
Version | 0.7.0-0
Commit Hash | a66fc4a
Committer Name | Darryl L. Pierce (mcpierce@gmail.com)
Time Of Last Commit | Jun 11, 2020, 12:12:03 AM
Commit Message | Changed how the next/previous issue is fetched [#329]
Built From Uncommitted Code? | false
Remote Origin | https://github.com/comixed/comixed.git

Now I open #1 and the next issue is the #0 instead of #2 :

I open #13 and the Previous issue is #9 instead of #0

Same i open #18 and the previous issue is always #9 instead #17

[mcpierce]

@BRUCELLA2 What DB are you using, the built in H2 supported one or an external one? Can you do the following SQL for me on your database:

SELECT id,issue_number,cover_date FROM comics WHERE series='Green Lantern' AND volume='2011' ORDER BY cover_date,issue_number;

and post that here?

[BRUCELLA2]

So, I use the built in H2. This is the result of the request :
133,1,2011-11-26
117,2,2011-12-31
113,3,2012-01-26
119,4,2012-02-26
100,5,2012-03-26
122,6,2012-04-30
106,7,2012-05-31
111,8,2012-06-26
125,9,2012-07-26
131,10,2012-08-26
103,11,2012-09-26
128,12,2012-10-26
109,0,2012-11-30
112,13,2012-12-31
126,14,2013-01-01
104,15,2013-02-26
123,16,2013-03-31
118,17,2013-04-30
115,18,2013-05-31
120,19,2013-06-01
134,20,2013-07-31
107,21,2013-08-31
130,22,2013-09-30
124,23,2013-10-31
135,23.1,2013-11-01
116,23.2,2013-11-01
127,23.3,2013-11-01
101,23.4,2013-11-01
105,24,2013-12-01
102,25,2014-01-01
129,26,2014-02-01
132,27,2014-03-31
110,28,2014-04-01
136,29,2014-05-31
121,30,2014-06-30
114,31,2014-07-31
108,32,2014-08-31

The order is correct. But it's not the same request but it's not the same request that's in the code. The ORDER BY is "ORDER BY cover_date,issue_number;" in this request and is "ORDER BY c.issueNumber,c.coverDate" in the code.

with the following query (like what's in the code regarding order by) : SELECT id,issue_number,cover_date FROM comics WHERE series='Green Lantern' AND volume='2011' ORDER BY issue_number, cover_date; i have this result :
109,0,2012-11-30
133,1,2011-11-26
131,10,2012-08-26
103,11,2012-09-26
128,12,2012-10-26
112,13,2012-12-31
126,14,2013-01-01
104,15,2013-02-26
123,16,2013-03-31
118,17,2013-04-30
115,18,2013-05-31
120,19,2013-06-01
117,2,2011-12-31
134,20,2013-07-31
107,21,2013-08-31
130,22,2013-09-30
124,23,2013-10-31
135,23.1,2013-11-01
116,23.2,2013-11-01
127,23.3,2013-11-01
101,23.4,2013-11-01
105,24,2013-12-01
102,25,2014-01-01
129,26,2014-02-01
132,27,2014-03-31
110,28,2014-04-01
136,29,2014-05-31
113,3,2012-01-26
121,30,2014-06-30
114,31,2014-07-31
108,32,2014-08-31
119,4,2012-02-26
100,5,2012-03-26
122,6,2012-04-30
106,7,2012-05-31
111,8,2012-06-26
125,9,2012-07-26

Order is not what we want.

(For information : i tested on my windows pc, with no dev tools and i have the same prob. I drop the db, make a new import and new scrap and the prob is always here. )

[mcpierce]

@BRUCELLA2 Ah, yeah, sorry for giving the wrong order. And I see what you're pointing out there. Though, strangely, the code as it is works correctly for me locally. But I'll update the code with the ordering swapped and see what behavior I get here.

[mcpierce]

@BRUCELLA2 Okay, I've tested this using my own Green Lantern comics in the same range and the change seems to have fixed the issue entirely, including showing issue 0 at the right point in the ordering. Please give the development build a try when it finishes.

[BRUCELLA2]

For everything works as expected now. I close this issue.