PKP Bugzilla – Bug 6125
OMP clean-up: clean up file upload controllers and grids
Last modified: 2011-08-25 11:08:30 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.
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: 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.
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
Refactored submission files grid: https://github.com/pkp/omp/commit/f3e76d54da55cba3fb1d24fccd34ba7019ede233
(Sorry, thats SubmissionWizardFilesGrid).
Added support for the "meta-data editing" step (except for artwork files) and a part of the "upload finished" step, see: https://github.com/pkp/pkp-lib/commit/818b1a209b646f14b3482ed77083caf553c32ad4 https://github.com/pkp/pkp-lib/commit/e5d5fc9354117312b81c35e66d1ec2d0601cdec5 https://github.com/pkp/omp/commit/76271a5dd67ed9a4a1c8be9e578dad00f800b539
Refactored submission details grid: https://github.com/pkp/omp/commit/c1cab32175e511eb2a93e787138df105620e8da4
Some cleanup of the last commit: https://github.com/pkp/omp/commit/de73cd4d3915d334943386164a5d4d409a5d500c
@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.
Recent commits are... JS handler + event API for grids: https://github.com/pkp/pkp-lib/commit/d17b83eadbe08d9919c75abfd034f470bbb1ca9f https://github.com/pkp/omp/commit/8b7a008b322df5a306b5c159200de66b36bca1ad Removed duplicate code from grid handler: https://github.com/pkp/pkp-lib/commit/95f14388495d4268471ff98c7218cf7140064e66 https://github.com/pkp/omp/commit/e2893949079dfe1e6c3392bec27c4b4b8a1801bc Removed duplicate code from category grid: https://github.com/pkp/pkp-lib/commit/1a52c00c7c7370ab845bd3ba7213884f1598ab8e Marked coding errors: https://github.com/pkp/omp/commit/9f7ac4967d2ce7c9d9d4d3426806996d87c69c42
Grid refresh event handling: https://github.com/pkp/pkp-lib/commit/a81bb26bbf8e4fd8f9879a7004bef4f2930acc3f
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').
Fixed bugs introduced by the commit linked in #11: https://github.com/pkp/omp/commit/ec7f7c7e9a3e6916d612dc14a5bf40762146a82a https://github.com/pkp/pkp-lib/commit/077a3519aea5c2805496e466931ae57d250d9dde
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: https://github.com/pkp/pkp-lib/commit/45c6cb2beb0ebfec9739430dc60f1def4f56bd65 https://github.com/pkp/omp/commit/c7ef808f1898c310b61b343c1a3bbec991a324f6
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 see https://github.com/pkp/pkp-lib/commit/3890a6798e51a331857229da90118ef217a4b8d7
Added newDataObject() method to submission file DAO, see: https://github.com/pkp/pkp-lib/commit/12933beaf267cc9e4b7a6a6d20d1b1de154f0fa7 https://github.com/pkp/omp/commit/67a217e6b19fe1f062a708b95c53ab1267903efd
Implemented artwork meta-data support, see https://github.com/pkp/omp/commit/887f7ce8c74fa52f4f85d21fa92ce9494c77188c
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
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
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
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 https://github.com/pkp/omp/commit/695767c5b94b6b48a467ee236ee61cd4d2ceaa22
a few other minor tweaks to the copyediting grid https://github.com/pkp/omp/commit/3d5409c4573cd624c073c7c69ae58b5e2b6aa486
Continued CE grid cleanup https://github.com/pkp/omp/commit/8ae6f90c1ed092fd2aa2f33f2c37c18d6309ef62
Fixed label for cases where files uploaded https://github.com/pkp/omp/commit/160f4a903034bc69c7df7fcb51fd46fc56665ba3
Delete action was using wrong api handler https://github.com/pkp/omp/commit/62857c12228c12b8fee7c3f2b5a2e235fe7d852f
When copying file to stage, expect file to come with different fileStage https://github.com/pkp/omp/commit/c514db45ca10ae649790d80b7d851a01b94c201b
rename locale keys for upload file actions https://github.com/pkp/omp/commit/5415ef4d7edc1370fe7194775f85b4f29fee58b1
FileManagementHandler can expect Monograph and StageId from AuthorizedContext and may be used without a fileStage passed https://github.com/pkp/omp/commit/3c5342e641b2a39d7eac734bda63f849ca1b9957
default to no selector when only one possible file to revise present https://github.com/pkp/omp/commit/3083f3f17aaf2f78bc9c5f85fb49cb5ddfcc9f80
Copying temporary files to a monograph file fix https://github.com/pkp/omp/commit/c7ccd0e5719844ef19d2a524ddd8f055d2628b1b
Allow getLatestRevisionByAssocId to include submission id https://github.com/pkp/pkp-lib/commit/b1170ed02fc4a7814c535cdd53dc6a807f2671c9
Default selection for C/E manage files modal https://github.com/pkp/omp/commit/936c7716c1fc5de1beb52a894d5d9f916ec9577d
Fixed Manage Final Draft Files modal etc. https://github.com/pkp/omp/commit/2560665e9e9fd9c10ecd6e6f040296678a07717f
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
dead code + typo fix https://github.com/pkp/omp/commit/357284dbd209d67f6ed605130d550205ef4e22b1
Closing -- I think the original entry is long since fixed.