gigascience / gigadb-website

Source code for running GigaDB
http://gigadb.org
GNU General Public License v3.0
9 stars 15 forks source link

Implement app service to provide access to dataset upload status #1751

Open rija opened 8 months ago

rija commented 8 months ago

User story

As a developer
I want all usage of upload status make call to methods or constants on a dedicated application service So that whenever statuses need to be changed or amended there is only on place in the codebase to do so

Acceptance criteria

Given there is a list of upload status When a subsystem needs to use "AuthorReview" as an upload status Then it makes a call to a service's method with "AuthorReview" as parameter that returns "AuthorReview"

Additional Info

this is about centralising the code that manipulate the upload status. There is a separate ticket #1755 for persisting them in the database

The idea is to ensure the logic of what is the id and what is the label of an upload status and the retrieval of the latter is handled in one place, a Yii2 service in gigadb/app/services/UploadStatusService.php

In this ticket a call to the service will return the same hardcoded label, so that current workflow still works. But once #1755 is implemented, it will return the Id.

Product Backlog Item Ready Checklist

Product Backlog Item Done Checklist

only1chunts commented 8 months ago

Just a pause for thought, could the upload statuses be a table in the database? Not sure what advantages that would have (if any) just throwing it up as a possibility. We will need both external and internal documentation of what each status means, so potentially being able to make subtle adjustment to the name for clarity in the future may be useful, meaning the consistent thing would be the database ID rather than the name. The database could include a description column.

On Tue, 12 Mar 2024, 01:34 Rija Ménagé, @.***> wrote:

User story

As a developer I want all usage of upload status make call to methods or constants in Dataset.php So that whenever statuses need to be changed or amended there is only on place in the codebase to do so

Acceptance criteria

Given conditions When event Then outcome

Given conditions When event Then outcome

Additional Info Product Backlog Item Ready Checklist

  • Business value is clearly articulated
  • Item is understood enough by the IT team so it can make an informed decision as to whether it can complete this item
  • Dependencies are identified and no external dependencies would block this item from being completed
  • At the time of the scheduled sprint, the IT team has the appropriate composition to complete this item
  • This item is estimated and small enough to comfortably be completed in one sprint
  • Acceptance criteria are clear and testable
  • Performance criteria, if any, are defined and testable
  • The Scrum team understands how to demonstrate this item at the sprint review

Product Backlog Item Done Checklist

  • Item(s) in increment pass all Acceptance Criteria
  • Code is refactored to best practices and coding standards
  • Documentation is updated as needed
  • Data security has not been compromised (with particular reference to the personal information we hold in GigaDB)
  • No deviation from the team technology stack and software architecture has been introduced
  • The product is in a releasable state (i.e. the increment has not broken anything)

— Reply to this email directly, view it on GitHub https://github.com/gigascience/gigadb-website/issues/1751, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABOB5GMIK72ZVVLWOIDZJOTYXXTLNAVCNFSM6AAAAABEQX7YU2VHI2DSMVQWIX3LMV43ASLTON2WKOZSGE3TSNZVGYZDMNI . You are receiving this because you are subscribed to this thread.Message ID: @.***>

rija commented 8 months ago

Hi @only1chunts

Could do. I once wondered why they were not when file types and dataset types are. I guess it kind of makes sense if they were considered a small number of unchanged statuses that are just a list of possible values for one field on datasets.

But the work on File Upload Wizard showed me that they represent a states chart of our organisation's important workflow (and the name upload_status may not be the best name anymore) and should probably be modelled and implemented as such.

That concern is only tangential to this issue though. This issue is to make sure they're is only on place in the codebase to handle them which fixes an immediate problem whereby DOIs could be minted in an inappropriate stage, as well as makes any future change to the status system (including #1616, #701 or implementing them in a database schema as we are discussing here) easier to do.

Also maybe a description column is not the only property we want to add to a state. I can think of (in)valid transition states (in or out) (that will reduce errors or incidents and protect correctness of our data), and wether we need to have shadow-representation of third party statuses (e.g: River Valley or OUP). We probably want to work out a DB schema (the Miro diagram seems like good starting point as input).

I guess I'm saying I like the idea, but it should probably be another ticket

rija commented 3 months ago

Hi @alli83,

In our codebase, the activities related to upload statuses are:

(I'm leaving out updating/adding/deleting upload statuses as those are exceptional events)

At the moment, those activities are implemented multiple times, in a slightly different ways (and sometimes incompletely), in various places in the codebase. You can search for the string upload_status to get a good idea of that.

This story is to consolidate the implementation of those activities in one service to avoid multiple slightly different implementations and use of hard-coded upload statuses, so to make it easy to change the activities or the backend implementation of the state-chart or amend the list of status with minimal impact to the rest of the codebase.

Because the upload status state-chart is quite foundational to the business's activities (not just the website), there is benefit in implementing a Yii2 Application component that provides helpers for the above activities as a service in its own project in gigadb/app/services instead of the legacy Yii1's protected/components, so that we can in the future make use of the service on other PHP projects (like GigaReview).

Once the service is written, then the parts of the codebase that has bespoke implementation of the above activities should be refactored to make a call to the new service's helpers.

In the context of this story, the service implementation should replicate existing behaviours/constraints, but should not make it hard to later add functionalities that we know are going to be needed like:

alli83 commented 2 months ago

Ok, based on PR#1999