Bug 6125

Summary: OMP clean-up: clean up file upload controllers and grids
Product: OMP Reporter: jerico <jerico.dev>
Component: GeneralAssignee: Alec Smecher <alec>
Status: RESOLVED FIXED    
Severity: normal CC: juan, mattcrider, pkp-support
Priority: P3    
Version: 1.0   
Hardware: PC   
OS: Linux   
Version Reported In: Also Affects:
Bug Depends on: 6127, 6128, 6223, 6231, 6233    
Bug Blocks: 5620    

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:
https://github.com/pkp/pkp-lib/commit/fbfddd6c6c266b7aff22bef5b31c683cbbef4ba3
https://github.com/pkp/omp/commit/452f3d31818cd31cba3483eb0892cbb83dcd6096
(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 :
https://github.com/pkp/pkp-lib/commit/ad0a7528c33a8f3145c02585cd4bb7c41b64742c
Comment 4 Matthew Crider 2011-01-04 17:18:42 PST
Refactored submission files grid: 
https://github.com/pkp/omp/commit/f3e76d54da55cba3fb1d24fccd34ba7019ede233
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:

https://github.com/pkp/omp/commit/de73cd4d3915d334943386164a5d4d409a5d500c
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:
https://github.com/pkp/pkp-lib/commit/a81bb26bbf8e4fd8f9879a7004bef4f2930acc3f
Comment 12 Matthew Crider 2011-01-31 13:45:59 PST
Added PageHandler JS, which gives us a URL redirection event handler.

https://github.com/pkp/pkp-lib/commit/bff37fc80bdfe606a4d571be8ab63fea4c27a003

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:
https://github.com/pkp/pkp-lib/commit/e4fb3a0ccb436a9dc72753f13c9d71290789cd54
https://github.com/pkp/omp/commit/a9b9b33f4f35c6cdc6baeda6e35b9334fd78231e

introduce monograph file type casting for artwork revision upload to work properly:
https://github.com/pkp/pkp-lib/commit/0b41c4b50b68035726441047a9ca4587aa7b41c4

rename JS files and templates consistently with classes, fix a few style and doc errors:
https://github.com/pkp/pkp-lib/commit/13f7cf68110a1d07c31da5e80ab40004b3d9b6af
https://github.com/pkp/omp/commit/905635a7bb8d849ebd59053bc2cba46538bdc30b
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:
https://github.com/pkp/pkp-lib/commit/482ee44c5bf32571f864fc38f4d9b1f66d1bd61d

fix bug in MonographFileManager when a genre is not given on upload:
https://github.com/pkp/omp/commit/c93aa763fe5f320280f2a8c69a92e1e247acb92c
Comment 22 jerico 2011-02-18 12:16:34 PST
correctly update grid after a revision has been deleted (via cancel), see
https://github.com/pkp/omp/commit/a7b0d52681faf8477f2e94d3309d70c5ae85c313
https://github.com/pkp/pkp-lib/commit/46367686090a8c8de593f02e1e001ed67a3a77b0
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

https://github.com/pkp/omp/commit/695767c5b94b6b48a467ee236ee61cd4d2ceaa22
Comment 26 Juan Pablo Alperin 2011-06-23 13:23:23 PDT
a few other minor tweaks to the copyediting grid
https://github.com/pkp/omp/commit/3d5409c4573cd624c073c7c69ae58b5e2b6aa486
Comment 27 Alec Smecher 2011-07-21 16:30:02 PDT
Continued CE grid cleanup
https://github.com/pkp/omp/commit/8ae6f90c1ed092fd2aa2f33f2c37c18d6309ef62
Comment 28 Alec Smecher 2011-07-25 12:20:03 PDT
Fixed label for cases where files uploaded
https://github.com/pkp/omp/commit/160f4a903034bc69c7df7fcb51fd46fc56665ba3
Comment 29 Juan Pablo Alperin 2011-08-05 23:50:06 PDT
Delete action was using wrong api handler
https://github.com/pkp/omp/commit/62857c12228c12b8fee7c3f2b5a2e235fe7d852f
Comment 30 Juan Pablo Alperin 2011-08-05 23:50:08 PDT
When copying file to stage, expect file to come with different fileStage
https://github.com/pkp/omp/commit/c514db45ca10ae649790d80b7d851a01b94c201b
Comment 31 Juan Pablo Alperin 2011-08-05 23:50:08 PDT
rename locale keys for upload file actions
https://github.com/pkp/omp/commit/5415ef4d7edc1370fe7194775f85b4f29fee58b1
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
https://github.com/pkp/omp/commit/3c5342e641b2a39d7eac734bda63f849ca1b9957
Comment 33 Juan Pablo Alperin 2011-08-05 23:50:08 PDT
default to no selector when only one possible file to revise present
https://github.com/pkp/omp/commit/3083f3f17aaf2f78bc9c5f85fb49cb5ddfcc9f80
Comment 34 Juan Pablo Alperin 2011-08-05 23:50:08 PDT
Copying temporary files to a monograph file fix
https://github.com/pkp/omp/commit/c7ccd0e5719844ef19d2a524ddd8f055d2628b1b
Comment 35 Juan Pablo Alperin 2011-08-07 23:00:10 PDT
Allow getLatestRevisionByAssocId to include submission id
https://github.com/pkp/pkp-lib/commit/b1170ed02fc4a7814c535cdd53dc6a807f2671c9
Comment 36 Alec Smecher 2011-08-10 14:25:01 PDT
Default selection for C/E manage files modal
https://github.com/pkp/omp/commit/936c7716c1fc5de1beb52a894d5d9f916ec9577d
Comment 37 Alec Smecher 2011-08-10 15:35:01 PDT
Fixed Manage Final Draft Files modal etc.
https://github.com/pkp/omp/commit/2560665e9e9fd9c10ecd6e6f040296678a07717f
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.
https://github.com/pkp/omp/commit/b5efef2239048eca1427f704b8f5f25a725595eb
Comment 39 Juan Pablo Alperin 2011-08-22 00:45:10 PDT
dead code + typo fix
https://github.com/pkp/omp/commit/357284dbd209d67f6ed605130d550205ef4e22b1
Comment 40 Alec Smecher 2011-08-25 11:08:30 PDT
Closing -- I think the original entry is long since fixed.