Bug 6128 - OMP clean-up: clean up the relation of book file types and MONOGRAPH_FILE_* constants.
OMP clean-up: clean up the relation of book file types and MONOGRAPH_FILE_* c...
Status: RESOLVED FIXED
Product: OMP
Classification: Unclassified
Component: General
1.0
PC Linux
: P3 normal
Assigned To: Matthew Crider
Depends on:
Blocks: 6125
  Show dependency treegraph
 
Reported: 2010-11-03 17:17 PDT by jerico
Modified: 2010-12-01 11:48 PST (History)
1 user (show)

See Also:
Version Reported In:
Also Affects:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description jerico 2010-11-03 17:17:00 PDT
We've got three orthogonal dimensions to characterize monograph files:
1) book file type, i.e. a categorization of the content of the file: manuscript, artwork, tables/figures, chapters, etc.
2) file stage, i.e. the location of the file within the editorial workflow: submission, review, final draft, fair copy, etc. , see MONOGRAPH_FILE_* in MonographFile.inc.php
3) mime type, i.e. the technical file type

Two of these dimensions have further hierarchy levels:
a) Book file types are grouped by book file type category (artwork vs. document)
b) Artwork files are further specified by artwork file type (e.g. picture, table, figure, ...)

There are a few inconsistencies in these definitions, though:
- MONOGRAPH_FILE_* constants describe file stages (see #2 above). There currently is a constant MONOGRAPH_FILE_ARTWORK which is not a file stage but (in that case) a book file type category (as opposed to "document", see a) above).
- Currently the book file type id (see #1 above) is saved into the assoc id field of the monograph file while the assoc type is not set. This is wrong in two respects: We always use the assoc id/assoc type to implement an "is-part-of" relationship and not a "has-a" relationship. The assoc id also should never be set without also setting the assoc type, otherwise the association will be ambivalent as soon as we introduce further association types for the object. book file type clearly has an aggregation relationship with monograph file. Therefore there should be an explicit accessor get/setBookFileTypeId().
- Why do we use the term "book file type" rather than "monograph file type"? In OMP there obviously is no conceptual distinction between a "book" and a "monograph". So the nomenclature is inconsistent. All BookFileType* classes must therefore be renamed to MonographFileType*.
- Why is there a book file type TABLES_AND_FIGURES when we also have an artwork type MONOGRAPH_ARTWORK_TYPE_FIGURE/TABLE? IMO figures/tables must either be monograph files or artwork files but cannot be both. Otherwise this is simply an instance of the conceptual weakness of the overly rigid artwork/document distinction that I mentioned in bug #6127, see #3 there.
- Only artwork files currently have a chapter association. Why do monograph galleys or monograph files not have this association? Wouldn't it be better to use the assoc id for this association? IMO all files can be either chapter or monograph specific.
- We should also keep in mind that chapters are by no means the only ways to divide monographs into parts. Can we be sure that we'll not soon get the requirement to enable users to divide books up into other parts with associated files?
Comment 1 Matthew Crider 2010-11-12 11:50:33 PST
Done.

https://github.com/pkp/omp/commit/408fe8a61c59c1dca6fa14c94d75d52f49112f3e
https://github.com/pkp/omp/commit/2528469afe16c304e0a2dd2a2f87716401bd7938
https://github.com/pkp/omp/commit/1bc12c308ae1664631b53daf6c71c754c684053d
https://github.com/pkp/pkp-lib/commit/cac7882163a8ad45d92dbafcf099ae5db8776f8b

I'm not sure what to do with chapters at the moment.  AFAIK, there is no specced interface for users (editors or authors) to associate files with chapters other than the free-text fields in the artwork file metadata tab.  I've opened bug 6190 so we can discuss this further.
Comment 2 jerico 2010-11-14 10:09:30 PST
Matt, this change introduced MonographFile::get/setMonographFileType() which seems to indicate a MonographFileType object type setter/getter. In reality you set/get the monograph file type *id* (which is correctly indicated by your variable name). IMO the setter should be get/setMonographFileTypeId(). Do you agree that it makes sense to change this? I promptly got confused when I fixed: https://github.com/pkp/omp/commit/089a65cc26ab7f099f6d60ae311da218bcd5d62f

BTW: Do you have a template (Preferences->PHP->Editor->Templates) for creating setters/getters in Eclipse? I'm using (for object-type getters/setters):

/**
 * Set the ${description}
 * @param $$${name} ${type}
 */
function set${namecaps}(&$$${name}) {
	$$this->_${name} =& $$${name};
}

/**
 * Get the ${description}
 * @return ${type}
 */
function &get${namecaps}() {
	return $$this->_${name};
}
${cursor}
Comment 3 jerico 2010-11-14 21:11:46 PST
Two more instances of the error reported in comment #2: https://github.com/pkp/omp/commit/b7bf00dfe547e6aa016d338be83344b5097b4d9f
Comment 4 Matthew Crider 2010-11-19 17:11:42 PST
Florian, as we discussed, and unless anyone else objects, I'll change the name of the monograph_file_type column (along with its accessors, and the monograph_file_type table itself) to 'genre' -- This gives us one out among the three 'types' (type, monograph_file_type, and file_type), and corresponds to the name of the Library of Congress classification of these types of entities.
Comment 5 jerico 2010-11-21 20:47:37 PST
Matt, I just got an idea for a potentially better name of the 'type' field... what about "use case" (get/setUseCase())? Isn't that exactly what is meant here? The use case of the file: review, copyediting, submission, etc.

What do you and the others think about that idea? If you like it you can rename that together with the monograph file type.
Comment 6 Matthew Crider 2010-11-22 09:40:32 PST
That sounds good Florian.  I'll go ahead with that change as well as the genre one.
Comment 8 Matthew Crider 2010-12-01 11:48:25 PST
Fixed some upgrade issues (values in file_stage column would get reset):
https://github.com/pkp/ocs/commit/a4a0ab9fdd63dbe61b1241cfc97c49b04c8a70f9
https://github.com/pkp/ojs/commit/88cad334d0a1e9917ae04c4bf37bf586860f89aa