Bug 6449 - resequencePublishedArticles called without sectionId from IssueManagementHandler
resequencePublishedArticles called without sectionId from IssueManagementHandler
Status: RESOLVED FIXED
Product: OJS
Classification: Unclassified
Component: Submissions and Publishing
2.3.5
All All
: P3 normal
Assigned To: PKP Support
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-02-27 11:34 PST by Chris Burdzy
Modified: 2011-03-08 15:28 PST (History)
1 user (show)

See Also:
Version Reported In:
Also Affects:


Attachments
Patch against OJS 2.3.4ish (1.82 KB, patch)
2011-03-02 08:59 PST, Alec Smecher
Details | Diff
Patch against OJS pre-2.3.5 (1.83 KB, patch)
2011-03-08 15:28 PST, Alec Smecher
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Burdzy 2011-02-27 11:34:46 PST
It seems to me that OJS 2.3.4 has a fundamental problem
with article ordering in the Table of Contents using arrows
in the Editor > Issue view.

I have a test installation of OJS 2.3.4 and I "published"
three test papers.
By pressing "up" and "down" arrows in Editor's view
of the Table of Contents several times, I created the following values
in the table "published_articles"

article_id 1 ; seq	-4 	
article_id 2 ; seq	1 	
article_id 3 ; seq	-3	

Then I pressed the "up" arrow by the third (last, lowest in order)
article. The new values of seq were

article_id 1 ; seq	-4 	
article_id 2 ; seq	-0.5	
article_id 3 ; seq	-3	

The new values of seq did not change the order of articles.

The above change of the value of "seq" can be 
explained by the following piece of code in
pages/editor/IssueManagementHandler.inc.php

   // Moving by up/down arrows
     $publishedArticle->setSeq(
       $publishedArticle->getSeq() + ($d == 'u' ? -1.5 : 1.5)

To be honest, the above code makes no sense to me,
from the logical point of view. In other words,
I do not see any problem with the coding. But the
mathematical algorithm behind the code makes no sense to me.
By the way, I am a mathematician. Perhaps I am missing something.

The real reason why I am looking into the guts of the
system is that I am trying to migrate EJP/ECP
http://www.math.washington.edu/~ejpecp/
to a new installation of OJS.
EJP/ECP is still based on OJS 1.1. This ancient
version of OJS has a field nOrder which is what
I desperately need in OJS 2.3.4. 
nOrder can be specified like page numbers
and in all cases at EJP/ECP it takes distinct consecutive
increasing integer values starting from 1 in a given
volume, encoding published paper number. Some volumes
in EJP have paper numbers but no page numbers
so I have to migrate nOrder in some way. I hoped
to add some code of my own to OJS 2.3.4 and
base it on "seq" but seq processing
code is clearly wrong. If anyone is willing to fix
seq, would it be too much to ask to restore nOrder
in some form, and especially its user-friendly
Editor interface for ordering articles using integers
(and not half-integers and negative numbers ?!?).

I love OJS but I will love it even more if nOrder
is restored.

Thanks for reading,
Chris
Comment 1 Alec Smecher 2011-02-28 08:54:08 PST
Chris, each change to the seq field should be followed by a resequencing of entries to use positive integers within that section and issue (see resequencePublishedArticles). The increment or decrement by 1.5 is used to move an entry between two other entries, and the subsequent resequencing will take care of making them integers again in the correct order. I'm not sure how those negative numbers got into the DB, but OTOH they shouldn't cause problems.
Comment 2 Chris Burdzy 2011-02-28 10:20:37 PST
Alec, Thanks for letting me know about resequencePublishedArticles. I found its description in your Doxygen documentation and it looks like it should work. But it does not work in my installation. I made some minimal changes to code in my installation and none of them came even close to seq processing. I can review my changes again but I wonder if you see any reason why resequencePublishedArticles would not be triggered? Is there a way for me to trace the problem? Thanks, Chris
Comment 3 Alec Smecher 2011-02-28 11:06:22 PST
Chris, the easiest way to debug in PHP is simply to use "echo" statements to output IDs and status info as the code executes. The call to resequencePublishedArticles happens pretty much immediately after the sequence field update. Of course, your error log is probably also a good place to look first.
Comment 4 Chris Burdzy 2011-03-01 18:55:23 PST
Please have a look at the following piece of code from
classes/article/PublishedArticleDAO.inc.php.
I used echo to determine that the loop 
for ($i=1; !$result->EOF; $i++)
did not execute. So I changed the loop to
for ($i=1; $i <= 5; $i++)
and once again I used echo to determine that the modified loop
did execute. But seq values are still half-integers
and negative numbers. Any suggestions?

Thanks,
Chris
        /**
         * Sequentially renumber published articles in their sequence order.
         */
        function resequencePublishedArticles($sectionId, $issueId) {
                $result =& $this->retrieve(
                        'SELECT pa.pub_id FROM published_articles pa, articles a WHERE a.section_id = ? AND a.article_id = pa.article_id AND pa.issue_id = ? ORDER BY pa.seq',
                        array($sectionId, $issueId)
                );

//              for ($i=1; !$result->EOF; $i++) {
                for ($i=1; $i <= 5; $i++) {
                        list($pubId) = $result->fields;
                        $this->update(
                                'UPDATE published_articles SET seq = ? WHERE pub_id = ?',
                                array($i, $pubId)
                        );

                        $result->moveNext();
                }

                $result->close();
                unset($result);

                $this->flushCache();
        }
Comment 5 Chris Burdzy 2011-03-01 20:34:11 PST
I did more tests. I replaced
for ($i=1; !$result->EOF; $i++)
with
for ($i=1; $i <= 5; $i++)
I also replaced 
array($i, $pubId)
with
array($i, $i)
Then the seq values became consecutive integers, as expected.
So the problem seems to be that $result and $pubId
do not get the right values.

Chris
Comment 6 Alec Smecher 2011-03-02 08:17:11 PST
Chris, the resequence operation only occurs within the provided $sectionId and $issueId. Are you sure you're not watching for changes to "seq" outside of the $sectionId and $issueId that is being resequenced?
Comment 7 Chris Burdzy 2011-03-02 08:24:39 PST
Alec, I am testing a new installation of OJS 2.3.4. There are only 5 articles:
http://depts.washington.edu/ejpecp/OJS2_3/ojs-2.3.4/index.php?journal=ejp
They are all published in only one section and one issue. The table "published_articles" lists all 5 published articles. Recall that the problem is not only that the values of seq are unusual (half-integers, negative numbers) but on the journal page, resequencing (sometimes) fails.
Comment 8 Alec Smecher 2011-03-02 08:59:06 PST
Created attachment 3444 [details]
Patch against OJS 2.3.4ish

Chris, try the patch below. The resequence function was being called with an empty section ID.
Comment 9 Chris Burdzy 2011-03-02 09:37:21 PST
(In reply to comment #8)
> Created attachment 3444 [details]
> Patch against OJS 2.3.4ish
> 
> Chris, try the patch below. The resequence function was being called with an
> empty section ID.

It works - thank you!  Chris
Comment 10 Alec Smecher 2011-03-08 15:28:41 PST
Created attachment 3453 [details]
Patch against OJS pre-2.3.5
Comment 11 Alec Smecher 2011-03-08 15:28:50 PST
(Committed against ojs-stable-2_3)