Bug 6245 - 2.2.x -> 2.3.3-3 upgrade "removes" submission notes
2.2.x -> 2.3.3-3 upgrade "removes" submission notes
Status: RESOLVED FIXED
Product: OJS
Classification: Unclassified
Component: Submissions and Publishing
2.3.4
All All
: P3 normal
Assigned To: PKP Support
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-11-24 09:41 PST by James MacGregor
Modified: 2011-02-23 14:41 PST (History)
3 users (show)

See Also:
Version Reported In:
Also Affects:


Attachments
article_notes table, pre-upgrade (878 bytes, text/xml)
2010-11-30 16:52 PST, James MacGregor
Details
notes table, before editing a note (1012 bytes, text/xml)
2010-11-30 16:52 PST, James MacGregor
Details
notes table, after editing a note (1001 bytes, text/xml)
2010-11-30 16:52 PST, James MacGregor
Details
Patch against OJS 2.3.3-3 (576 bytes, patch)
2010-11-30 17:25 PST, Alec Smecher
Details | Diff
Patch against OJS 2.3.3-3 (585 bytes, patch)
2010-11-30 17:33 PST, Alec Smecher
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description James MacGregor 2010-11-24 09:41:32 PST
Reported by John: http://pkp.sfu.ca/support/forum/viewtopic.php?f=8&t=6863&p=26576#p26576, and I've replicated. After upgrading, any previously created notes aren't listed in the submission notes page, but they can be accessed by going directly to the submission note's Edit page (eg. http://git/ojs224/index.php/demo/editor/submissionNotes/1/edit/1).  

On a possibly related topic -- has the note storage location changed by any chance? If I edit the note, my changes seem to be saved according to the UI; but the note in the article_notes table doesn't appear to be updated.
Comment 1 Alec Smecher 2010-11-25 12:07:59 PST
James, the storage for article notes has moved from "article_notes" to the generalized "notes" table. Could you pass me a database dump for that table so I can see what happened during upgrade?
Comment 2 James MacGregor 2010-11-25 16:36:09 PST
Hi Alec, 

Actually, scratch that -- after upgrade, the article_notes table is completely empty, and the note appears as it should in the notes table as it should. If I go directly to the Edit interface and edit the note, my edit is reflected in the notes table, and the note actually then appears in the Submission Notes listing.
Comment 3 Alec Smecher 2010-11-25 19:44:25 PST
James, does this mean that after the upgrade the notes are somehow in a bad state, but that editing them and saving them corrects that state? Or do I misunderstand?
Comment 4 James MacGregor 2010-11-30 16:52:06 PST
Created attachment 3371 [details]
article_notes table, pre-upgrade
Comment 5 James MacGregor 2010-11-30 16:52:31 PST
Created attachment 3372 [details]
notes table, before editing a note
Comment 6 James MacGregor 2010-11-30 16:52:48 PST
Created attachment 3373 [details]
notes table, after editing a note
Comment 7 James MacGregor 2010-11-30 16:53:09 PST
(In reply to comment #3)
> James, does this mean that after the upgrade the notes are somehow in a bad
> state, but that editing them and saving them corrects that state? Or do I
> misunderstand?

It looks like the context_id field isn't being set properly on upgrade. All note context_ids are set to null after the upgrade is complete, but switch to 1 when I edit and save a note. Notes only display when the context_id is set to 1. The attached table  xml dumps should be fairly self-explanatory; you'll see that in notes_post-edit.xml only one context_id field is 1 -- that's because I only edited the one note.
Comment 8 Alec Smecher 2010-11-30 17:25:19 PST
Created attachment 3374 [details]
Patch against OJS 2.3.3-3
Comment 9 Alec Smecher 2010-11-30 17:26:35 PST
James, could you try the attached patch? You may need to patch bug #6220 first, before this patch will apply cleanly. It'll need to be applied before you run the upgrade process.
Comment 10 Alec Smecher 2010-11-30 17:33:40 PST
Created attachment 3375 [details]
Patch against OJS 2.3.3-3

Oops, corrected patch.
Comment 11 Alec Smecher 2010-11-30 17:40:52 PST
Also fixed for OCS (doesn't affect any released versions). See https://github.com/pkp/ocs/commit/b48b31e3cd92c7dc8faa349d9cb0802c1e0be721
Comment 12 James MacGregor 2010-11-30 18:24:06 PST
(In reply to comment #9)
> James, could you try the attached patch? You may need to patch bug #6220 first,
> before this patch will apply cleanly. It'll need to be applied before you run
> the upgrade process.

Hi Alec, that appears to do the trick. I'm guessing that if the upgrade process has already been completed, users running into this issue can just set the context_id fields in the notes table to 1?
Comment 13 James MacGregor 2010-11-30 18:25:01 PST
(In reply to comment #12)
> (In reply to comment #9)
> > James, could you try the attached patch? You may need to patch bug #6220 first,
> > before this patch will apply cleanly. It'll need to be applied before you run
> > the upgrade process.
> 
> Hi Alec, that appears to do the trick. I'm guessing that if the upgrade process
> has already been completed, users running into this issue can just set the
> context_id fields in the notes table to 1?

Oops -- just looked at the patch -- that would be, they set the context_id to the relevant journal ID.
Comment 14 Alec Smecher 2010-11-30 18:25:33 PST
That should be possible using:

UPDATE notes n, articles a SET n.context_id = a.journal_id WHERE a.article_id = n.assoc_id;
Comment 15 Matthew Crider 2011-02-21 17:24:39 PST
In upgrade from 2.2.3: 

[data: dbscripts/xml/upgrade/2.3.3_update.xml]
ERROR: Upgrade failed: DB: Unknown column 'notes.context_id' in 'field list'

The notes.xml schema doesn't seem to have a context_id, which I assume should?  Also, there still appears to be an article_notes definition in ojs_schema.xml.  And to be a real nitpick, the table comment in notes.xml refers to the groups table.
Comment 16 Alec Smecher 2011-02-23 14:41:52 PST
The context column isn't necessary and was removed from other parts of the system. I've removed it from the upgrade process too:

https://github.com/pkp/ojs/commit/bdf98143afc0d41fa72c6f7f9e21ef7e8b358f65

Also removed unnecessary article_notes table definition and corrected notes table comment (in separate commits).