- Issue created by @phenaproxima
- First commit to issue fork.
- πͺπΈSpain penyaskito Seville π, Spain πͺπΈ, UTC+2 πͺπΊ
I've created a new branch based on @phenaproxima's as I plan to work on this during the weekend, but don't want to disrupt any work he might have on his local or maybe we had different understandings on how to do this.
- πͺπΈSpain penyaskito Seville π, Spain πͺπΈ, UTC+2 πͺπΊ
I've got to a point where even the previous UI is mostly kept using the config actions.
I'd need to discuss with Adam to continue, as I'm not sure how he intents to use the existing conditions.Also is not clear if we want to keep the existing task help texts/links, or if description should be a rich text. If that's the case, the api or modules implementing the api might need to include text formats?
For completion, I like the idea that we could have a "CompletionPlugin" (e.g. one of them being a checkbox), but not sure how that could be/extend the condition api. Also the 2.x branch allow to store that in config or state if I read the code correctly, and that might be a feature we want to implement in the future.
I added some @todos on places that might need discussion, and lots of hacky code will need to be edited/remove, but if the scope of this was the config entity, I'm pretty satisfied with what I got to. Just needed to get a bit further to ensure that the existing implementation could be achieved with the new one.
- πͺπΈSpain penyaskito Seville π, Spain πͺπΈ, UTC+2 πͺπΊ
In the meantime did some progress on providing blocks on the checklist-dashboard-block branch (might be in a different computer next week, had to push it somewhere)
- πΊπΈUnited States phenaproxima Massachusetts
Here's what I've come up with so far in https://git.drupalcode.org/issue/checklistapi-3474237/-/tree/3474237-cre... -- it's only sketched in, and by no means ready to commit, but I thought I'd explain the design choices here.
Long story short is that this MR adopts very well-established patterns for modern Drupal development: config entities, and plugins.
In my view, Checklist API has two "pillars": storing checklist definitions (i.e., what are the tasks in this checklist? How are they grouped? etc.) and managing checklist state (which tasks are completed?). The first pillar is implemented using a config entity; the second pillar is implemented with plugins.
The config entity is called Checklist, and has a corresponding ChecklistInterface. Like all config entities, it is exportable, importable, and translatable. Checklist entities have absolutely nothing to do with state storage; they simply are lists of tasks and task groups. Checklist entities do not define specific routes by which they can be accessed, because that's a pain in the butt and introduces caching-related weirdness; there will be only one canonical route for a checklist, probably something like
/checklist/CHECKLIST_ID
. (A standard path alias could be created to visit a checklist via an alternate path.)The tasks are represented with a simple value object, called Task. Tasks contain a runtime reference to their defining checklist, a basic string identifier, a human-readable title and description, an identifier of which group they're in (if any), and an optional set of conditions under which they are considered fulfilled. (More on that in a bit.) Tasks also don't know what state they're in.
That brings us to state management. This is done with plugins, which implement a new ProgressInterface. Progress plugins are responsible for storing the state of tasks in a checklist. They can tell you whether a task is complete, explicitly mark a task as complete or incomplete, or reset the progress of a whole checklist. The only link between the Checklist entities and the progress plugins is that the checklist will specify which progress plugin it wants to use. (This is a very common pattern in core -- MediaType and Workflow entities are two examples that leap to mind.)
Out of the box, there are two progress plugins: State and Config. The only real difference between these two is that State is environment-specific, and Config persists across environments. External code could implement more types of progress plugins if they wanted.
Code that wants to interrogate the progress of a checklist can use the plugin manager for the progress plugins, which is called ProgressManager. It contains methods to report how many tasks in a checklist are completed (either as an integer or a percentage). ProgressManager is the intersection between checklists (which don't know their state), and the progress plugins (which know about the state of checklist tasks, but don't know anything else about the checklist itself). Plugin managers having type-specific superpowers is also a common pattern (for example, BreakpointManagerInterface, MailManagerInterface, BlockManagerInterface, and others). I did NOT create an interface for ProgressManager because it felt superfluous, but I'm not against adding one if that's desirable.
Now let's talk about those conditions I mentioned. Drupal has the concept of a "condition plugin", which is essentially just a way to dynamically get a boolean value based on...some conditions in the system. I know that they are used in core by the block system, but I'm not sure where else; nevertheless, they are an established and supported core API. I think it makes sense for Task objects to be able to optionally define a set of conditions that, if all of them are fulfilled, mean that the task itself is considered complete. I implemented this in a basic way. Tasks that don't define any conditions must be manually marked as complete, but tasks that do define conditions can be automatically determined to be complete. ProgressManager has an explicit method to facilitate this.
- πΊπΈUnited States phenaproxima Massachusetts
phenaproxima β changed the visibility of the branch 3474237-entity-type to hidden.
- πΊπΈUnited States phenaproxima Massachusetts
phenaproxima β changed the visibility of the branch 3474237-entity-type to active.
- Merge request !14Sketch in checklist entity type and a unit test β (Merged) created by phenaproxima
- Issue was unassigned.
- Status changed to Needs review
2 months ago 5:06pm 18 September 2024 - πΊπΈUnited States phenaproxima Massachusetts
OK - I think I'm happy with this.
This MR only sets up the data model. It has nothing to do with state management or progress storage. We'll do that in subsequent issues.
This sets up a pretty standard config entity, called
Checklist
, which generally follows the structure defined byhook_checklistapi_checklist_info()
andcallback_checklistapi_checklist_items()
. Tasks and links are represented by immutable value objects that can be upcast from, and downcast to, arrays for export and storage purposes. There is a very basic API to modify the config entity -- you can add task groups and tasks, but currently there is no way to remove any of those things. (We can add those in other issues, if needed.) Config actions are also natively supported, with the trade-off being that support for Drupal older than 10.3 is dropped.Virtually all of these changes have kernel test coverage, too!
- πΊπΈUnited States phenaproxima Massachusetts
phenaproxima β changed the visibility of the branch 3474237-create-a-new to hidden.
- πΊπΈUnited States phenaproxima Massachusetts
phenaproxima β changed the visibility of the branch checklist-dashboard-block to hidden.
- πΊπΈUnited States phenaproxima Massachusetts
phenaproxima β changed the visibility of the branch 3474237-config-entity-with-test to hidden.
- Status changed to RTBC
2 months ago 7:11pm 18 September 2024 - πͺπΈSpain penyaskito Seville π, Spain πͺπΈ, UTC+2 πͺπΊ
Love this kick-off for the 3.0.x branch.
- Status changed to Needs review
2 months ago 7:18pm 18 September 2024 - Status changed to RTBC
2 months ago 7:27pm 18 September 2024 - πΊπΈUnited States phenaproxima Massachusetts
Thanks @penyaskito for the sharp thinking on the
string|\Stringable
stuff. After a cursory search, I can't find any precedent in core so I think we should just dostring
for now. I've made that change and therefore I'm restoring RTBC! - First commit to issue fork.
-
traviscarden β
committed d3e2d95a on 3.0.x authored by
phenaproxima β
Issue #3474237 by phenaproxima, penyaskito, traviscarden: Create a new...
-
traviscarden β
committed d3e2d95a on 3.0.x authored by
phenaproxima β
- πΊπΈUnited States traviscarden
Nice! Merged. Good work, guys; this is exciting. π
Automatically closed - issue fixed for 2 weeks with no activity.