[FOLIO-2088] RFC - handling of cross-module URL's Created: 07/Jun/19 Updated: 03/Jun/20 |
|
| Status: | Open |
| Project: | FOLIO |
| Components: | None |
| Affects versions: | None |
| Fix versions: | None |
| Type: | New Feature | Priority: | TBD |
| Reporter: | John Coburn | Assignee: | Unassigned |
| Resolution: | Unresolved | Votes: | 2 |
| Labels: | RFC | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original estimate: | Not Specified | ||
| Issue links: |
|
||||||||||||||||||||||||||||||||||||||||
| Sprint: | |||||||||||||||||||||||||||||||||||||||||
| Development Team: | Stripes Force | ||||||||||||||||||||||||||||||||||||||||
| Description |
|
The Problem: throughout platform-core, ui-modules use links to detail records within other modules, hard-coding urls complete with query parameters. This creates a fragile coupling between modules, as one module can't change its own urls for sake of the other modules using them.
----------------------- **** ---------------------------
stripes: {
actions: {
paths : {
'user-view': '/users/view/:userid',
'user-edit': '/users/view?layer=edit&id=:userid',
'user-create': '/users/view?layer=create',
'loan-view': '/users/:userid/loans/:loanid'
'fine-create': '/users/:userid/feesfines/create'
}
}
}
Contrived example. This shape is up for discussion. The keys for paths could be broken out separate objects per-recordType... but this shape lends to simple string lookup rather than some kind of lodash-ish get style lookup. import { getActionURL, getActionList } from @folio/stripes/core; /* getActionURL() would return a url string with the values of the 2nd parameter appropriately * slotted in. If 'user-edit' is unregistered, or if string-substitution isn't completed (configuration not * filled out) the function returns null and the consuming module can * figure out what it needs to do with that... */ userEditURL = getActionURL('user-edit', { id: someIdVar }); /* getActionList - Utility function, gets a read-only set of the registered action paths in the * system and their configuration objects... so a developer can check which cross-module actions are * available within a platform and what keys are needed for their configuration. */ getActionList(); // => [ { name: 'users-edit', configurationKeys:'id, ...' }, ...] I imagine all configurationKeys would be required. |
| Comments |
| Comment by Aliaksei Chumakou [ 10/Jun/19 ] |
|
My thoughts - we need a separate E2E testing tool. like https://www.cypress.io/ or https://www.protractortest.org/, so we can write meaningful acceptance tests with cross-module navigation. And tests will fail in case of module changed it's url. I don't think BigTest supports such testing, but if it does - we can do it within BigTest. |
| Comment by md331 (Inactive) [ 11/Jun/19 ] |
|
I think we should probably decide whether we want to solve the issue of module replacement with this as well. Eg, I manage the Licenses app, which has the ability to link a user to a license. A user's name is hyperlinked to their record in the Users app. If a tenant doesn't use ui-users but instead uses myorg-folio-users, how does the Licenses app know that? Where should it get the paths? If we didn't already need the concept of record types for
|
| Comment by John Coburn [ 11/Jun/19 ] |
|
Aliaksei Chumakou I believe the issue that you mention could be resolved if we had testing that occurred at the platform level atop the isolated module level. Modules themselves cannot take for granted the existence of any other ui-modules in the system. The platform, however, knows which apps it is including, and could be expected to test that sort of cross-functionality. The details of that sort of setup are a little outside of the scope of this discussion, but if there were a centralized way of pulling the recordTypes/URL's exposed by existing UI modules, those could actually be stubbed out in BigTest at the module level and the 'presence' of those links tested in isolation of the module. md331 I feel like inter-operational matters are best handled at the platform level - and the closest we have to that is stripes-core - so beseeching stripes-core (or some other centralized thing) is the way the paths would be gotten.
|
| Comment by Mike Taylor [ 12/Jun/19 ] |
The Licenses app neither knows nor cares whether the tenant has deployed vanilla ui-users or ui-mikes-awesome-users. The responsibility is on ui-mikes-awesome-users to provide the same URL endpoints as ui-users: that's what it means to be a drop-in replacement for ui-users. Does that mean my users module has to use all the same paths as ui-users? I suppose not: there are (at least notionally) a documented set of paths that constitute the API of the users module — or, more precisely, of the "users app" interface, which ui-users happens to be one implementation of. So when ui-licences links to /users/view/123 that doesn't mean "make ui-users show user 123", it means "show user 123 by means of whatever users module we happen to have". |
| Comment by md331 (Inactive) [ 12/Jun/19 ] |
|
That's what I would expect, Mike Taylor. My question is really how do we achieve that? It seems like this is another issue that requires a record-type-keyed registry. So my hope is that if we do it right, we can do it once and tackle both the card/association and URL problems at the same time. |
| Comment by Mike Taylor [ 12/Jun/19 ] |
|
I'm not sure they're the same problem. With our computer-scientist hats on, we could go off and design a machine-readable UI constraint language for describing the incoming-URL contracts that UI modules implement — or, come to think of it, couldn't we just use RAML? But maybe in practice, it suffices to add a few lines in the ui-users README.md saying "This UI modules responds to the following URIs: (insert list). Any drop-in replacement must do likewise." To me, that feels like a very different thing from the kind of solution we want for the cards. |
| Comment by md331 (Inactive) [ 12/Jun/19 ] |
I mentally discarded this solution because I'm really not a fan I mean, it doesn't even solve the issue outlined in this issue's description since it's "a fragile coupling between modules, as one module can't change its own urls for sake of the other modules using them." |
| Comment by Mike Taylor [ 12/Jun/19 ] |
|
No, of course one module can't change its own URLs. Neither can any module of any kind change its contract once it's committed to it. That's what "contract" means. |
| Comment by md331 (Inactive) [ 12/Jun/19 ] |
|
If we create an API then it can... |
| Comment by Mike Taylor [ 12/Jun/19 ] |
|
Why create another API when there is one already? |
| Comment by John Coburn [ 12/Jun/19 ] |
|
An API I picture for a non-users module that wants to link to a users module would go something like this... import { getSharedURL } from `@folio/stripes/core`; const url = getSharedURL('user.detail', { id: userIdVar }); { url && <Link to={url} /> Where ui-users (or whichever appropriate user-handling module) has appropriately indicated
stripes: {
types: {
user: {
paths: {
detail: '/users/view/:id'
}
}
}
}
This puts the API at the stripes level, not directly coupling modules and makes room for graceful fallback that's up to the consuming module. All a ui-module has to do, if it does update its URL paths, is update those shared paths in its package.json - so it doesn't have to worry about updating its documentation or bumping its major version, or whispering to other module maintainers to "please update the url's you've used for me." |
| Comment by Mike Taylor [ 12/Jun/19 ] |
|
¯_(ツ)_/¯ I've got no objection to what John proposes. I just don't see what it gets us. Sometimes, and extra layer of indirection is just another place for things to go wrong and another bit of code that needs documenting, testing and maintaining. Still, if you want to build this, I won't argue against it. |
| Comment by Jason Skomorowski [ 13/Jun/19 ] |
|
Either way we have interfaces that replacement modules need to implement. The current (poorly documented) interface is the set of public URLs that we want to be reliably linkable (eg. it doesn't have to be the whole URL scheme). So a candidate to replace users has to have view/:id, edit/:id, etc. (note that we'd want these relative to the app's route) And what you propose is to add another layer such that modules are free to form their own routes and the interface becomes they must map a route to user.view, one to user.edit etc. I understand presently we're changing URLs around because we're moving from heavily overlay based stuff, which was kinda fiddly with a bit too much resting in the query string, but do we actually anticipate whatever URL scheme we settle on to be that onerous as an interface or change very often afterwards? Long term, I do get that it's probably not great to dictate (some of) the routes must be the same for all modules you might choose to mount at /whatever, but I suspect it's not urgent and that we might be better off designing this mapping alongside the breadcrumbs/app sections stuff that somewhat relates to it. |
| Comment by John Coburn [ 13/Jun/19 ] |
|
Conventionally, it's difficult to dictate an entire pathname - and I would even feel weird about expecting a module/recordType to have a certain shape of URL... users/view/:id users/:id/loans/view/:loanid users/:id/accounts/view/:accountid Similar issue with the overlay-based query-string. |