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 5231 - Remove internal author arrays and functions from Submission classes & fix value objects that instantiate DAOs
Remove internal author arrays and functions from Submission classes & fix val...
Status: REOPENED
Product: OJS
Classification: Unclassified
Component: General
2.4.x
PC All
: P5 normal
Assigned To: PKP Support
: 5912 5913 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-03-17 11:58 PDT by Matthew Crider
Modified: 2011-12-12 09:18 PST (History)
4 users (show)

See Also:
Version Reported In:
Also Affects: OCS 2.3.x, OJS 2.3.x


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Matthew Crider 2010-03-17 11:58:55 PDT
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.
Comment 2 Juan Pablo Alperin 2010-04-09 19:56:17 PDT
moving to OJS as reminder to backport
Comment 3 Alec Smecher 2010-07-20 12:44:38 PDT
Deferring.
Comment 4 Matthew Crider 2010-10-08 11:01:09 PDT
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).
Comment 5 jerico 2010-11-02 15:40:01 PDT
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.
Comment 6 jerico 2010-11-02 15:40:56 PDT
*** Bug 5912 has been marked as a duplicate of this bug. ***
Comment 7 jerico 2010-11-02 15:45:10 PDT
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.
Comment 8 jerico 2010-11-02 15:46:57 PDT
Same for OMP's Author::getLocalizedUserGroupName()
Comment 9 jerico 2010-11-02 15:51:00 PDT
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.
Comment 10 Juan Pablo Alperin 2011-03-20 23:45:35 PDT
marking as "also affects" for backporting detection in OCS and OJS.
Comment 11 Juan Pablo Alperin 2011-03-20 23:46:15 PDT
*** Bug 5913 has been marked as a duplicate of this bug. ***
Comment 15 Alec Smecher 2011-11-18 08:02:44 PST
Matt, there's still a case where this isn't working. See https://github.com/pkp/pkp-lib/commit/9d72544cc5d89c592f6707101e1403defa4c69aa#commitcomment-729003 for details.
Comment 16 Matthew Crider 2011-11-23 13:53:59 PST
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.
Comment 17 Alec Smecher 2011-11-23 13:58:03 PST
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>.