We are moving to Git Issues for bug tracking in future releases. During transition, content will be in both tools. If you'd like to file a new bug, please create an issue.

Bug 6125 - OMP clean-up: clean up file upload controllers and grids
OMP clean-up: clean up file upload controllers and grids
Product: OMP
Classification: Unclassified
Component: General
PC Linux
: P3 normal
Assigned To: Alec Smecher
Depends on: 6127 6128 6223 6231 6233
Blocks: 5620
  Show dependency treegraph
Reported: 2010-11-03 15:40 PDT by jerico
Modified: 2011-08-25 11:08 PDT (History)
3 users (show)

See Also:
Version Reported In:
Also Affects:


Note You need to log in before you can comment on or make changes to this bug.
Description jerico 2010-11-03 15:40:18 PDT
The file upload controllers and grids need cleaning up. As the specs for them came in small steps they've not been designed with a "big picture" in mind. They need to be refactored now that we can see where we're heading. Quite some copy&paste needs to be replaced with a good class hierarchy to remove the code duplication we have right now between controllers.
Comment 1 Matthew Crider 2010-11-08 15:21:30 PST
I'll work on the UML for this, but will wait until our next cleanup phase to implement.
Comment 2 jerico 2010-12-25 23:16:51 PST
Completely restructured the file upload wizard (view: based on the new JS framework, backend: better OO, duplicate code removal), prototypical implementation for file uploader and revision confirmation, see the FairCopyFileGridHandler:
(Oups - commit messages point to 6294 which is the introduction of the JS framework - sorry.)

I'll continue with this handler. It will become the reference implementation for the clean-up of all file grids.
Comment 3 jerico 2010-12-27 21:34:27 PST
split upload and revision into separate forms, replaced remaining upwards references in forms with events, correctly handle file deletion on cancel, introduced specific wizard for file upload :
Comment 4 Matthew Crider 2011-01-04 17:18:42 PST
Refactored submission files grid: 
Comment 5 Matthew Crider 2011-01-04 17:20:01 PST
(Sorry, thats SubmissionWizardFilesGrid).
Comment 7 Matthew Crider 2011-01-06 15:20:11 PST
Refactored submission details grid: https://github.com/pkp/omp/commit/c1cab32175e511eb2a93e787138df105620e8da4
Comment 8 Matthew Crider 2011-01-07 13:24:02 PST
Some cleanup of the last commit:

Comment 9 jerico 2011-01-09 23:15:38 PST
@Matt: As promised, the file grid class is now done. As a side effect I've completely done away with the fair copy file row class. The same probably works for most other grids. Can you check that for the grids you changed? Unfortunately I discovered today that cell providers are also a big mess. I cannot fix them immediately, though, because I've to fix bug #6233 first.

Above all, my latest commit sets up the event API for grids and fixed the last few methods in the submission file grid handler. This means that now all file grids can be refactored with the submission files grid handler as a base class. Please don't forget that we'll have to create further base classes on top of the submission file grid handler or grid handler to represent the other basic grid types. There's a lot more to do there to remove all copied code.

There are still a few minor rough edges in the file upload and file grid. Here's my current issues list:

Upload tab:
- When uploading the first file then after the upload the continue button should either be de-activated or (probably better) have the same effect as the "start upload" button.
- Changing the genre / revised file of an uploaded file should be possible even after choosing a file for upload.
- We still have to save the user group of the uploader of the file and show the correct columns for "uploader" and "auditor" in the grid (depends on #6233). This has never been implemented.

Metadata tab:
- Artwork meta-data support does not yet work. The existing artwork form contains duplicate code and must be refactored as a sub-class of the normal meta-data form.
- Usability: The first form input should be focused by default.

Finished tab:
- I've not yet implemented the "new file" button.

Internal stuff:
- I should better align the naming of some PHP forms/handlers with their corresponding JS forms/handlers.
- I should make all events private by default (i.e. internal to their respective handler), events that should bubble outside the handler are declared with Handler::publishEvent() (needs to be documented in the Wiki)

I'll successively fix these remaining issues over the next days. None of these issues should be in the way of the general file grid refactoring. Files can now also be uploaded for testing.
Comment 11 Matthew Crider 2011-01-27 11:34:57 PST
Grid refresh event handling:
Comment 12 Matthew Crider 2011-01-31 13:45:59 PST
Added PageHandler JS, which gives us a URL redirection event handler.


This is attached to the main content div of the page in the header (in OMP, thats 'div.main').
Comment 14 jerico 2011-02-04 05:54:11 PST
Matt, re your comment #12. This change does not work for me:
1) The page handler is not attached to the main div.
2) The page handler is missing in minifiedScripts.tpl.
3) I'm missing some method on the server side that allows us to trigger the new event.
4) I'm getting compile warnings for the handler (PageHandler.js:53: WARNING - parameter string does not appear in $.pkp.controllers.PageHandler.prototype.redirectToUrl's parameter list).

I'll fix this because I need this code.
Comment 16 jerico 2011-02-04 14:36:43 PST
Some changes that blocked bug #6373, see https://github.com/pkp/omp/commit/08ba058dbc4c4cff0f531dd03cb25e599556c348
Removed accidentally committed galley files, see https://github.com/pkp/omp/commit/84a6518b2525a43e06ec5866f0f0d81637f055d4
Comment 17 jerico 2011-02-16 09:07:20 PST
Fixed a few bugs in the upload form:
- when uploading the first file then after the upload the continue button is activated in the upload step
- changing the genre / revised file of an uploaded file should be possible even after choosing a file
- canceling no longer closes the wizard

see https://github.com/pkp/pkp-lib/commit/3890a6798e51a331857229da90118ef217a4b8d7
Comment 19 jerico 2011-02-16 14:43:52 PST
Implemented artwork meta-data support, see https://github.com/pkp/omp/commit/887f7ce8c74fa52f4f85d21fa92ce9494c77188c
Comment 20 jerico 2011-02-17 17:22:12 PST
implement "new file" button in last upload step, see:

introduce monograph file type casting for artwork revision upload to work properly:

rename JS files and templates consistently with classes, fix a few style and doc errors:
Comment 21 jerico 2011-02-17 18:03:02 PST
remove the _currentStep variable from the Wizard in favor of the _currentTab variable which is already in TabbedHandler.js:

fix bug in MonographFileManager when a genre is not given on upload:
Comment 22 jerico 2011-02-18 12:16:34 PST
correctly update grid after a revision has been deleted (via cancel), see
Comment 23 Juan Pablo Alperin 2011-06-21 16:17:57 PDT
alec, can this be closed as result of your grid work?
Comment 24 Alec Smecher 2011-06-21 16:24:00 PDT
May as well leave open until the grid spreadsheet is done -- I'm tagging my changes against this entry. (Unless there's another more appropriate one?)
Comment 25 Juan Pablo Alperin 2011-06-23 13:22:55 PDT
added delete signoff action to copyeditingfilessignoff grid

Comment 26 Juan Pablo Alperin 2011-06-23 13:23:23 PDT
a few other minor tweaks to the copyediting grid
Comment 27 Alec Smecher 2011-07-21 16:30:02 PDT
Continued CE grid cleanup
Comment 28 Alec Smecher 2011-07-25 12:20:03 PDT
Fixed label for cases where files uploaded
Comment 29 Juan Pablo Alperin 2011-08-05 23:50:06 PDT
Delete action was using wrong api handler
Comment 30 Juan Pablo Alperin 2011-08-05 23:50:08 PDT
When copying file to stage, expect file to come with different fileStage
Comment 31 Juan Pablo Alperin 2011-08-05 23:50:08 PDT
rename locale keys for upload file actions
Comment 32 Juan Pablo Alperin 2011-08-05 23:50:08 PDT
FileManagementHandler can expect Monograph and StageId from AuthorizedContext and may be used without a fileStage passed
Comment 33 Juan Pablo Alperin 2011-08-05 23:50:08 PDT
default to no selector when only one possible file to revise present
Comment 34 Juan Pablo Alperin 2011-08-05 23:50:08 PDT
Copying temporary files to a monograph file fix
Comment 35 Juan Pablo Alperin 2011-08-07 23:00:10 PDT
Allow getLatestRevisionByAssocId to include submission id
Comment 36 Alec Smecher 2011-08-10 14:25:01 PDT
Default selection for C/E manage files modal
Comment 37 Alec Smecher 2011-08-10 15:35:01 PDT
Fixed Manage Final Draft Files modal etc.
Comment 38 Juan Pablo Alperin 2011-08-11 21:00:03 PDT
default filename to the original file name, change default behaviour of revision selection dropdown, and mark notes as read by the uploader.
Comment 39 Juan Pablo Alperin 2011-08-22 00:45:10 PDT
dead code + typo fix
Comment 40 Alec Smecher 2011-08-25 11:08:30 PDT
Closing -- I think the original entry is long since fixed.