Regularise Repository usage throughout app.

[jdickey]

If you look at the test code for Prolog::UseCases::ProposeEditContribution, at the test dummy set up for the Contribution repository, you see that it has one method that no other test repo has: a #create method.

Why is this important? Because the code that calls it neither knows nor cares what type of object it's being handed back; it just trusts that it can do what it's being asked to do with it; classic Ruby "duck typing". This is something we never remembered to do with our other test code, and that had an extremely unpleasant side-effect: it's made Prolog::Core::User and Prolog::Core::Article explicit dependencies of both the tests and the code.

This should not and need not be, as the ProposeEditContribution tests show; the "repository" is simply an array of arrays of field values, which is all that's needed for the purposes of the test. Explicitly creating a Prolog::Entities::EditContribution::Proposed instance (which is another bit of possibly premature detail presently in the codebase) just makes the code more brittle.

If we establish a standard repository pattern across the (currently incompatible) test dummies/stubs used in the prolog_use-cases Gem, especially a #create method, we move prolog_core from being a hard run-time dependency of this Gem to being a softer, development-only dependency. If we use test doubles and dummies in place of "real" objects, so long as we maintain compatibility with the actual core entities, we can eliminate the explicit dependency completely. That would decouple the two Gems, and make future (including very-near-future) development of either much smoother.

Listed as a "bug" because we should have known better than to not do this from the beginning.

---

This would also be a good time to think about separating query objects out from repositories. Queries run against (and thus depend on) things-that-work-like Repositories; code that executes queries (anything presently touching the Repository/dummy instances now) deals with injected query objects rather than injected repository objects. This has two benefits: first, it makes it far easier to identify, and later optimise, interactions between a use case or entity ("client code") and an underlying persistence layer, and secondly, it further isolates client code from knowledge of or dependence on implementation details of persistence. This is another why-didn't-we-get-it-through-our-skulls-way-back-when "doing it more approximately 'right'" concept. :persevere:

[jdickey]

RegisterNewMember touches the repository (i.e., provides specs for possible query objects) in three places:

1. The last line of the #call method adds a new entity to the repository. This can be satisfied by the regularised repository API, and so needs no change.
2. #name_available? expects the repository to have a #query_user_by_name method. This is a prime candidate for a query object to mediate between intention-revealing semantics from the use case on the one hand, and the newly-regularised repository API on the other; and
3. #new_entity calls the repository's #create method and expects it to hand back whatever entity-ish thing it created.

All three of those depend on the repository; one of them has extremely idiosyncratic expectations of the interface supported by that repository, and thus presents a case for a query object — coming right up.

[jdickey]

Actually, query objects in the sense that I was envisioning them (heavily inspired by their implementation in ROM) are pretty orthogonal to form objects, which (a) we're already heavily invested in, and (b) are probably easier for new Rails-infected devs to wrap their mind around, so we're going to at least defer everything below the grey bar in the opening comment.

[jdickey]

What do we mean by a "regularised" Repository API? The discussion of this has been broken out into a Wiki page.

[mitpaladin]

update(entry)?

are we sure #delete(entry) wont be needed?

[jdickey]

#add (and #save) work for either new or updated records. We haven't needed #delete yet; when we do, we'll #add it. :stuck_out_tongue_winking_eye:

[jdickey]

Commit 8868f42 may have "done and tested" the "one-and-only public method" used in the form object as such (`#wrap_contribution_with), and added validation to block changing article content when we're not logged in as a Member, but the use case isn't quite with the programme yet; it still does a lot of its own validation, which ought to be (and imminently will be) ripped out. If "your best code is the code that you don't need to write,", then your worst code is the code you've just obsoleted that's still gunking up the works anyway.

It's not quite that simple, of course; the in-the-use-case validation used the spun-up-for-purpose ContentValidation class, which needs to be instead plugged into (and likely modified to suit) the form-object validation.

And then we truly will be done with this! :clap:

[jdickey]

And, with Commit d44c1f5, we've finally done all we will/can do to/with/for the ProposeEditContribution use case; time to move on. :clap:

(Sometimes we think there really should be a :wild_applause: or :roaring_celebration: emoji.)

Hang on, we just noticed we've an article_repo parameter that is never used. Protip for detecting such: if you think you're done, and your only instantiation of a CUT method parameter in your tests is defined as Object.new, you're probably not using it for anything. Delay, snatched from the dripping jaws of completion!

This indicates we have a problem: either

1. our use case is expected to persist the article (which is also passed in) to the repository, after successfully validating the proposal and updating the article's body content to indicate the location and affected content of the proposal; or
2. that persistence is to be handled outside the use case, unlike our other use cases that persist their entities on successful creation.

"I'll take Door Number One, Monty", as they sometimes say on the TV game show; cue Issue #29.

[jdickey]

After opening Issue #29, and noticing that the one-and-only remaining Virtus model (as opposed to value object) in the app is in Prolog::Services::ContentSelection::FormObject, we're ready to merge the PR and close this issue.