-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Overhaul of link expansion docs #2763
Conversation
docs/link-expansion.md
Outdated
reversal in link expansion and has the role of determining which editions | ||
will need re-presenting to the content store as a result of an update to a | ||
document. | ||
[dependency resolution](dependency-resolution.md). This is the opposite of link expansion. When a document is updated, Publishing API works out which documents link to the updated document, and presents the linked documents to Content Store. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to add something here about the limitations of dependency resolution in cases where the cardinality of the links has changed or the ordering of the links, but I couldn't actually rationalise what the limitation is.
Theoretically when an edition has a new link added to it, dependency resolution should run to ensure that any content items which link to the edition are presented to content store with their latest expanded links. Is that not how things work in reality?
Obviously when it's a new document we can't have linked to it yet so dependency resolution can't work, but that can be managed through Publishing apps (though at this point if Publishing apps are doing callbacks to update related items on create, they might as well do them on update and delete as well, so dependency resolution isn't abstracting away a lot of complexity for them).
beefec7
to
23e5860
Compare
This commit makes the differences between link set links and edition links more explicit. It also explains why publishing applications might use one or the other of those categories of link and why each of them exists. It also attempts to explain dependency resolution in a bit more detail. This commit also includes a large number of edits for clarity.
23e5860
to
80ce934
Compare
80ce934
to
291991b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this 🎖️ . These are much clearer and easier to understand now.
docs/link-expansion.md
Outdated
|
||
They are typically used for taxonomy based links and aren't related to the | ||
specific content of an edition. | ||
Edition links should be the default approach to link creation, because link set links have the disadvantage of applying to all editions and locales of a document, which is generally not what is required in most use cases. However, edition links have a substantial limitation, which is that recursive link expansion is not applied to them. If a document needs to access data from documents more than one link away, then link set links must be used. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you link to the section on recursive link expansion here, just in case the reader isn't sure what that is.
|
||
They do not follow a draft/published workflow, once added they will apply to | ||
editions of a document on both draft and live content stores. | ||
There was an attempt to allow recursive expansion of edition links, but we weren't able to complete it. See <https://github.com/alphagov/publishing-api/pull/2605>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's worth noting that we found another (albeit convoluted) way of solving the problem we faced, without needing recursive link expansion. We probably could've got the recursive expansion working if we'd dedicated a lot more time to it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've found this a fair bit clearer than the previous iteration, and it's nicely structured. Thanks for making these changes 👍
This commit makes the differences between link set links and edition links more explicit. It also explains why publishing applications might use one or the other of those categories of link and why each of them exists.
It also attempts to explain dependency resolution in a bit more detail.
This commit also includes a large number of edits for clarity. There's a particularly notable one in the section about edition state which was very ambiguous about whether it was discussing the linked edition or the linking edition - worth checking particularly closely for accuracy.
Trello: https://trello.com/c/iIVbEEWO