PKP Bugzilla – Bug 6125
OMP clean-up: clean up file upload controllers and grids
Last modified: 2011-08-25 11:08:30 PDT
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.
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.
I'll work on the UML for this, but will wait until our next cleanup phase to implement.
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.
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 :
Refactored submission files grid:
(Sorry, thats SubmissionWizardFilesGrid).
Added support for the "meta-data editing" step (except for artwork files) and a part of the "upload finished" step, see:
Refactored submission details grid: https://github.com/pkp/omp/commit/c1cab32175e511eb2a93e787138df105620e8da4
Some cleanup of the last commit:
@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:
- 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.
- 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.
- I've not yet implemented the "new file" button.
- 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.
Recent commits are...
JS handler + event API for grids:
Removed duplicate code from grid handler:
Removed duplicate code from category grid:
Marked coding errors:
Grid refresh event handling:
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').
Fixed bugs introduced by the commit linked in #11:
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.
Fixed problems mentioned in previous comment:
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
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
Added newDataObject() method to submission file DAO, see:
Implemented artwork meta-data support, see https://github.com/pkp/omp/commit/887f7ce8c74fa52f4f85d21fa92ce9494c77188c
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:
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:
correctly update grid after a revision has been deleted (via cancel), see
alec, can this be closed as result of your grid work?
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?)
added delete signoff action to copyeditingfilessignoff grid
a few other minor tweaks to the copyediting grid
Continued CE grid cleanup
Fixed label for cases where files uploaded
Delete action was using wrong api handler
When copying file to stage, expect file to come with different fileStage
rename locale keys for upload file actions
FileManagementHandler can expect Monograph and StageId from AuthorizedContext and may be used without a fileStage passed
default to no selector when only one possible file to revise present
Copying temporary files to a monograph file fix
Allow getLatestRevisionByAssocId to include submission id
Default selection for C/E manage files modal
Fixed Manage Final Draft Files modal etc.
default filename to the original file name, change default behaviour of revision selection dropdown, and mark notes as read by the uploader.
dead code + typo fix
Closing -- I think the original entry is long since fixed.