TheProlog / prolog-use_cases

Use-case layer for Meldd/Prolog application.
0 stars 0 forks source link

Make consistent and DRY up use-case Result classes. #66

Closed jdickey closed 8 years ago

jdickey commented 8 years ago

We currently have five use-case Result classes with an :errors attribute. Three of these use a dry-types type definition we've created, Types::ErrorArray:

Two others do not, but present no reason for being inconsistent with the others:

This issue specifies that the latter two Result classes' :errors attributes use the same Types::ErrorArray type as the first three. This will enable a base class, proposed to be named Prolog::Entities::Result::Base, to be extracted from the newly-compatible Result classes, that supplies the :errors attribute itself along with query methods such as #success? and #failure?. This will allow each of the existing Result classes to be simplified, removing redundant code (and specs).

jdickey commented 8 years ago

Next question: why are there only five Result classes?

jdickey commented 8 years ago

With regard to that "why are there only five Result classes":

Use-Case Class Uses Result? Has compliant :errors attribute? Notes
AcceptSingleProposal Yes (see Notes) Yes Uses Prolog::Entities::Result::Response class for Result.
AuthoriseContributionResponse Yes Yes (none)
ProposeEditContribution Yes Yes (none)
PublishNewArticle No No Still uses deprecated form object class.
QueryArticleProposedContributions Yes Yes (none)
RegisterNewMember No No Still uses deprecated form object class.
RejectSingleProposal Yes Yes Uses Prolog::Entities::Result::Response class for Result.
RespondToSingleProposal Yes Yes (see Notes) Equivalent declaration; does not use the Types::ErrorArray typedef.
RetrieveArticle No No Still uses deprecated form object class.
SummariseArticlesByAuthor Yes No Old-style Result without Dry::Types.
SummariseContent No No Theoretically failure-proof.
SummariseOwnContribHistory Yes No Currently simple Types::Strict::Array
ValidateSelection No No Still uses deprecated form object class.

Per that table, we have several problems:

Form Objects Must Die

These classes still use old-style form objects:

- [ ] PublishNewArticle (see Issue #68) - [ ] RegisterNewMember (see Issue #69) - [ ] RetrieveArticle (see Issue #70) - [ ] ValidateSelection (see Issue #71)

Close, But No Cigar

These classes should require very simple changes to get with the current protocol:

Get With the Protocol!

These classes don't return a Result value object instance from #call, and thus have nonstandard error handling (and don't use the old form-object protocol for errors):

* SummariseContent (see Issue #72)

jdickey commented 8 years ago

PublishNewArticle is one of our earliest use cases, and it shows.

😞

jdickey commented 8 years ago

We've created a spec for an updated Publish a New Article use case, replacing the one implemented in Gem Release 0.3.0. It has been broken out as Issue #68.

It is our feeling that this should not be "slipstreamed" into a Gem release addressing other features (such as the current 0.12.0); hence, Issue #68 is not assigned to the current in-work milestone.

jdickey commented 8 years ago

Actually, on further review, each of the use cases listed above under Form Objects Must Die has the same compatibility problems noted in the immediate preceding comment for Publish a New Article.

Therefore, each of them needs specs for an updated (reimplemented) use case, and issues corresponding to #68 for each of them. They will be addressed in individual, separate, Gem releases after 0.12.0.

jdickey commented 8 years ago

Each of the "problem" Use Cases listed above, with the exception of the (now-completed) Close, but No Cigar items, will require a separate specification for reimplementation (which already exists for Publish a New Article, see Issue #68). Each will require a new, separate Gem release, as they break the existing implementations' interfaces.

We again note our previous argument that updated or replaced implementations of classes in the prolog-use_cases Gem should have their existing implementations removed from that Gem in favour of external Gems (which prolog-use_cases should then depend on). The rationale is that if a class changes once, it could change again in future, and isolating the changing code from the not-yet-revised prolog-use_cases helps isolate potential ramifications of those changes. It's entirely possible that, under this scenario, prolog-use_cases will eventually become a "meta-Gem" that simply relies on other Gems which have replaced its former functionality.

Be that as it may, with the deferring of these use cases' updates, it appears that we can proceed with the extraction of a shared Prolog::Entities::Result::Base class, as described in the opening comment of this issue.

The end is nigh!

jdickey commented 8 years ago

Note that the RespondToSingleProposal class has a current-convention-compatible errors array but is not really conventional in total; for instance, it has provision to somehow query for a response from the UI (!). Epic fail in need of reimplementation.

Happily, no; as noted in the Road Map and the previous spec for the Use Case, this Use Case has been replaced and superseded by three other Use Cases since Gem Release 0.11.0. The RespondToSingleProposal use case, its tests, and its support code, should be removed.