5231 2010-03-17 11:58:00 -0700 Remove internal author arrays and functions from Submission classes & fix value objects that instantiate DAOs 2011-12-12 09:18:55 -0800 1 1 1 Unclassified OJS General 2.4.x PC All REOPENED P5 normal --- 1 mattcrider pkp-support alec jerico.dev juan pkp-support OCS 2.3.x OJS 2.3.x 18876 mattcrider 2010-03-17 11:58:55 -0700 The Submission class contains an array of authors and various functions to maintain this array from within the submission class. This code is redundant and authors should be maintained exclusively in the AuthorDAO. For instance, $monograph->getAuthors(), if it exists at all, should be an interface to $authorDao->getAuthorsByMonographId(). Anything necessary to manipulate authors (e.g. getting/setting/deleting/reordering) is already available in the AuthorDAO. This can be done in OMP without modifying pkp-lib classes, but should be ported to the other apps after the next release. 18911 mattcrider 2010-03-19 12:53:49 -0700 Commits that will help backport this to OJS and OCS: http://github.com/pkp/omp/commit/15e78686b4737dc4a1f6a50d927452c24f277fb9 http://github.com/pkp/omp/commit/7df49ecd3414157ff6e25f177a4169afa41d4899 19100 juan 2010-04-09 19:56:17 -0700 moving to OJS as reminder to backport 20137 alec 2010-07-20 12:44:38 -0700 Deferring. 21556 mattcrider 2010-10-08 11:01:09 -0700 Need to remove calls to the AuthorDAO from within Monograph. Consider leaving calls to AuthorDAO in Paper and Article classes to maintain backwards-compatibility (but mark as deprecated). 21896 jerico.dev 2010-11-02 15:40:01 -0700 Also see the comments in bug #5912: value objects like MonographFile and its decendants (i.e. ArtworkFile) and potentially also corresponding classes in other apps (e.g. ArticleFile, etc.) cannot instantiate their own DAO. What's especially ugly is that MonographFile, etc. seem to "instantiate themselves", see getFile(), getFilePath(), etc. which is the wrong way round. The DAO should instantiate the value object and not the value object the DAO to then retrieve itself via it's own ID using a different object instance. One example: getFilePath() $monographDao =& DAORegistry::getDAO('MonographDAO'); $monograph =& $monographDao->getMonograph($this->getMonographId()); $pressId = $monograph->getPressId(); We should simply be able to say $this->getPressId() to retrieve the press id rather than having to "instantiate ourselves". If this doesn't work then there's probably a conceptual error elsewhere. 21898 jerico.dev 2010-11-02 15:40:56 -0700 *** Bug 5912 has been marked as a duplicate of this bug. *** 21899 jerico.dev 2010-11-02 15:45:10 -0700 Same for ArtworkFile::getPermissionFile() - Should be populated when retrieving the file from the DAO. See the different descendants of Submission for good examples how this should be done. 21900 jerico.dev 2010-11-02 15:46:57 -0700 Same for OMP's Author::getLocalizedUserGroupName() 21901 jerico.dev 2010-11-02 15:51:00 -0700 Re: getLocalizedUserGroupName() - Should be either getUserGroup() or getUserGroupId() rather than a localized user group name. Maybe this shouldn't even be part of the Author object but be queried from the UserGroupDAO when needed or be some method like UserGroupDAO::getObjectByUserId(), etc. Otherwise we'd have to go on with similar accessors for all dependent entities that happen to be 1:n-related to authors. 23303 juan 2011-03-20 23:45:35 -0700 marking as "also affects" for backporting detection in OCS and OJS. 23305 juan 2011-03-20 23:46:15 -0700 *** Bug 5913 has been marked as a duplicate of this bug. *** 23900 mattcrider 2011-06-03 10:44:09 -0700 Posted to my github for review: https://github.com/mcrider/ocs/commit/0e04f03effde19ed65a6a6c5a6e2aa829fe3e0c0 https://github.com/mcrider/ojs/commit/7c9240104d100b7adb3adc3cbaf4b575dbc0cd82 https://github.com/mcrider/omp/commit/f489fd247f685e85d97fab19d85dbe011ccd8e59 https://github.com/mcrider/pkp-lib/commit/4b61a74861274103c2d18a20540b47362fa8843e 23902 mattcrider 2011-06-03 14:49:23 -0700 Second pass, removed some convenience methods from Submission class: https://github.com/mcrider/pkp-lib/commit/731d395cb2f895254c692aec533429a0ccfeae08 https://github.com/mcrider/omp/commit/f489fd247f685e85d97fab19d85dbe011ccd8e59 https://github.com/mcrider/ojs/commit/909e6aa6b49247c31153eb96b7a57bdf0ebfe22d https://github.com/mcrider/ocs/commit/1a738c7461c1bf36918bcc2899eeb905d7e9f17c 23903 mattcrider 2011-06-03 16:15:05 -0700 Committed to official. Closing. https://github.com/pkp/ocs/commit/9facaaf81c911231dff53f2d54223a25a64102ab https://github.com/pkp/ojs/commit/70a6e682d7d471162101e91b2bab47995f32148a https://github.com/pkp/omp/commit/d08b3386dc4411ae18d69b76b398eb04611ad581 https://github.com/pkp/pkp-lib/commit/9d72544cc5d89c592f6707101e1403defa4c69aa 25864 alec 2011-11-18 08:02:44 -0800 Matt, there's still a case where this isn't working. See https://github.com/pkp/pkp-lib/commit/9d72544cc5d89c592f6707101e1403defa4c69aa#commitcomment-729003 for details. 25889 mattcrider 2011-11-23 13:53:59 -0800 Alec, is there a document that shows how metadata adapters work? This should be a one-line fix, but I have no idea how to reproduce the issue to confirm the fix. 25890 alec 2011-11-23 13:58:03 -0800 It's not clear from the stack trace how it's being invoked; might as well follow up with Florian on the github comment. The filter stuff is part of the metadata framework, documented at <http://pkp.sfu.ca/wiki/index.php/Metadata_and_Filter_Framework>.