Skip to content

Refactor file models #865

Description

@mauromsl

The File models have grown a little hairy over time:

  • The core.models.File model holds a reference to submission.Article objects and exposes methods to interact with the article. Could use an ArticleFile model instead (or defer this responsibilities to core.models.Galley).
  • The core.models.File model exposes the vestigial attribute is_galley, should be removed and its references replaced with an instance of Galley
  • Deleting a core.models.File object leaves an orphan file in the filesystem, we should either remove the file or just flag the File object as deleted
  • core.models.Galley holds a key to a Article and File, while File also holds a key to Article. The two could get out of sync. Either we make Galley inherit from a File base model OR Galley holds a key to an ArticleFile but not to an Article`
  • core.models.Galley has is_remote and remote_file attributes which are interdependent. The presence of a remote_file should indicate if a galley is remote (could expose a property named is_remote preserve the original interface)
  • core.models.File also has is_remote and remote_url which can get out of sync with Galley. Should only exist on one of the two models.
  • core.models.Galley and core.models.File expose a method to build a path for serving an article. Both are currently in use.
  • core.models.File objects cannot access the file itself, unless you somehow know where the article lives in the filesystem (under articles, under journals, under plugins...). A File instance should be able to retrieve the file on its own. Ideally, we would use Django's ``FileSystemStorage` for this, as we do with other files in the codebase.
  • core.models.File exposes instance methods get_file_path and get_article_path where get_file_path takes an argument article_id whilst already holding in its state an article id. This means that a file instance could serve the path to any article file, not just itself.
  • core.models.File exposes an instance method named preprint_path. This should be on a different model, as not all instances of File are preprints
  • core.models.FileHistory is clever but it is tightly coupled with Article model. Should work for any file

Metadata

Metadata

Assignees

No one assigned

    Labels

    InfrastructureFor issues that are infrastructure but not quite devops; refactoring, DB, framework etc.enhancementAdd or improve something on an existing featurejaneway 2.0refactoringThis issue is a refactor of existing code.t-shirt epicA very large piece of work that will need to be broken up into smaller issues

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions