Skip to content
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

ELEMENTS-1216: add route resolver to nuxeo-routing-behavior #271

Merged
merged 1 commit into from Nov 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
96 changes: 74 additions & 22 deletions ui/nuxeo-routing-behavior.js
Expand Up @@ -71,22 +71,26 @@ export const RoutingBehavior = {
_computeUrlFor() {
return function(...args) {
if (this.router) {
const route = args[0];
const baseUrl = this.router.baseUrl || '';
if (route.startsWith('/')) {
return baseUrl + route;
const [route, ...params] = args;
if (!route) {
return '';
nelsonsilva marked this conversation as resolved.
Show resolved Hide resolved
}
if (!this.router[route]) {
console.error(`Could not generate a url for route ${route}`);
return;
const baseUrl = this.router.baseUrl || '';
let path;
if (typeof route === 'object') {
Gabez0r marked this conversation as resolved.
Show resolved Hide resolved
path = this._routeEntity(...args);
} else {
if (route.startsWith('/')) {
return baseUrl + route;
}
if (!this.router[route]) {
console.error(`Could not generate a url for route ${route}`);
return;
}
path = this.router[route].apply(this, params);
}
const params = Array.prototype.slice.call(args, 1);
return (
baseUrl +
(baseUrl.endsWith('/') ? '' : '/') +
(this.router.useHashbang ? '#!' : '') +
this.router[route].apply(this, params)
);
const base = `${baseUrl}${this.router.useHashbang ? `${baseUrl.endsWith('/') ? '' : '/'}#!` : ''}`;
return `${base}${base.endsWith('/') || path.startsWith('/') ? '' : '/'}${path}`;
}
};
},
Expand All @@ -98,19 +102,67 @@ export const RoutingBehavior = {
_computeNavigateTo() {
return function(...args) {
if (this.router) {
const route = args[0];
const [route, ...params] = args;
const baseUrl = this.router.baseUrl || '';
if (route.startsWith('/')) {
this.router.navigate(baseUrl + route);
let path;
if (typeof route === 'object') {
path = this._routeEntity(...args);
} else {
if (route.startsWith('/')) {
this.router.navigate(baseUrl + route);
}
if (!this.router[route]) {
console.error(`Could not navigate to a url for route ${route}`);
}
path = this.router[route].apply(this, params);
}
if (!this.router[route]) {
console.error(`Could not navigate to a url for route ${route}`);
}
const params = Array.prototype.slice.call(args, 1);
this.router.navigate(this.router[route].apply(this, params));
this.router.navigate(path);
} else {
console.error('No router defined');
}
};
},

_routeEntity(...args) {
if (args.length === 0) {
return;
}
const [obj, ...params] = args;
if (typeof obj !== 'object') {
throw new Error(`cannot resolve route: "${obj}" is not a valid entity object`);
}
let entityType = obj['entity-type'];
if (!entityType) {
// XXX Sometimes we don't have the entity type. For example, on nuxeo-document-storage, we were not storing it.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we fix this at nuxeo-document-storage level so we store the entity-type and add it in case it's missing on stored documents so we can do without this hack here ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could certainly fix this for newly added documents, but I'm not quite sure how to fix this for already existing documents in a reliable way, since we just forward the value of the inner iron-localstorage. We do have a get method which we use on some elements, but I don't think it would cover all use cases, specially those that rely on data binding.

Alternatively, we could try to do something with a new private property to hold the "fixe" set of documents, or we could try to run something when the value changes to fix the documents, but I'm not sure it is worth the effort?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can try to fix this on nuxeo-home, which is the only place by default where we generate URLs from docs stored on local storage, AFAIK. But I'm not sure I'd call this robust.

// In such cases, we'll assume we are dealing with a document if it has both a path and a uid properties.
if (obj.path && obj.uid) {
entityType = 'document';
} else {
throw new Error(`cannot resolve route: object does not have an "entity-type"`);
}
}
let routeKey =
Nuxeo &&
Nuxeo.UI &&
Nuxeo.UI.config &&
Nuxeo.UI.config.router &&
Nuxeo.UI.config.router.key &&
Nuxeo.UI.config.router.key[entityType];
nelsonsilva marked this conversation as resolved.
Show resolved Hide resolved
let fn = this.router[entityType];
if (entityType === 'document') {
routeKey = routeKey || 'path';
if (obj.isProxy || obj.isVersion) {
routeKey = 'uid';
}
// XXX we keep `routeKey === 'path' && this.router.browse` to keep compat routers that override the document
// document` method and do not know how to handle paths
fn = (routeKey === 'path' && this.router.browse) || fn;
nelsonsilva marked this conversation as resolved.
Show resolved Hide resolved
}
routeKey = routeKey || 'id'; // let `id` be the default key
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎵 let id be 🎵 let id be ...

const routeVal = obj[routeKey];
if (!routeVal) {
throw new Error(`invalid router key: ${routeKey}`);
}
return fn(routeVal, ...params);
},
};
50 changes: 47 additions & 3 deletions ui/test/nuxeo-document-suggestion.test.js
Expand Up @@ -40,11 +40,20 @@ function getSuggestions(suggestionWidget, timeout = 1000) {

// Mock router
const router = {
browse: () => '#',
user: () => '#',
group: () => '#',
browse: (path) => path,
document: (uid) => `/doc/${uid}`,
};

function setNuxeoRouterKey(entityType, value) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remark: we really need a window.Nuxeo.UI.config.set('...') or something...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

window.Nuxeo = window.Nuxeo || {};
window.Nuxeo.UI = window.Nuxeo.UI || {};
window.Nuxeo.UI.config = window.Nuxeo.UI.config || {};
window.Nuxeo.UI.config.router = window.Nuxeo.UI.config.router || {};
window.Nuxeo.UI.config.router.key = window.Nuxeo.UI.config.router.key || {};
window.Nuxeo.UI.config.router.key = window.Nuxeo.UI.config.router.key || {};
window.Nuxeo.UI.config.router.key[entityType] = value;
}

suite('nuxeo-document-suggestion', () => {
let server;

Expand Down Expand Up @@ -92,6 +101,21 @@ suite('nuxeo-document-suggestion', () => {
expect(entries[0].childElementCount).to.be.equal(1);
expect(entries[0].children[0].nodeName.toLowerCase()).to.be.equal('a');
expect(entries[0].children[0].textContent).to.be.equal('Some Title');
expect(entries[0].children[0].href.endsWith('/default-domain/workspaces/toto')).to.be.true;
expect(widget._resolveDocs.calledOnce).to.be.equal(true);
});

test('Should be able to resolve document and display its title when resolving with document UID', async () => {
setNuxeoRouterKey('document', 'uid');
widget.value = 'existingDocId';
await flush();
const entries = await getSuggestions(widget);
setNuxeoRouterKey('document'); // reset document key to undefined
expect(entries.length).to.be.equal(1);
expect(entries[0].childElementCount).to.be.equal(1);
expect(entries[0].children[0].nodeName.toLowerCase()).to.be.equal('a');
expect(entries[0].children[0].textContent).to.be.equal('Some Title');
expect(entries[0].children[0].href.endsWith('/doc/existingDocId')).to.be.true;
expect(widget._resolveDocs.calledOnce).to.be.equal(true);
});

Expand Down Expand Up @@ -151,6 +175,26 @@ suite('nuxeo-document-suggestion', () => {
expect(entries[0].childElementCount).to.be.equal(1);
expect(entries[0].children[0].nodeName.toLowerCase()).to.be.equal('a');
expect(entries[0].children[0].textContent).to.be.equal('Some Title');
expect(entries[0].children[0].href.endsWith('/default-domain/workspaces/toto')).to.be.true;

expect(entries[1].childElementCount).to.be.equal(1);
expect(entries[1].children[0].nodeName.toLowerCase()).to.be.equal('span');
expect(entries[1].children[0].textContent).to.be.equal('deletedDocId');

expect(widget._resolveDocs.calledOnce).to.be.equal(true);
});

test('Should be able to resolve the existing documents and reconciliate when resolving with UID', async () => {
setNuxeoRouterKey('document', 'uid');
widget.value = ['existingDocId', 'deletedDocId'];
await flush();
const entries = await getSuggestions(widget);
setNuxeoRouterKey('document'); // reset document key to undefined
expect(entries.length).to.be.equal(2);
expect(entries[0].childElementCount).to.be.equal(1);
expect(entries[0].children[0].nodeName.toLowerCase()).to.be.equal('a');
expect(entries[0].children[0].textContent).to.be.equal('Some Title');
expect(entries[0].children[0].href.endsWith('/doc/existingDocId')).to.be.true;

expect(entries[1].childElementCount).to.be.equal(1);
expect(entries[1].children[0].nodeName.toLowerCase()).to.be.equal('span');
Expand Down
173 changes: 173 additions & 0 deletions ui/test/nuxeo-routing-behavior.test.js
@@ -0,0 +1,173 @@
/**
@license
(C) Copyright Nuxeo Corp. (http://nuxeo.com/)

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
import { fixture, html } from '@nuxeo/testing-helpers';
import { mixinBehaviors } from '@polymer/polymer/lib/legacy/class.js';
import '@nuxeo/nuxeo-elements/nuxeo-element.js';
import { RoutingBehavior } from '../nuxeo-routing-behavior.js';

function setNuxeoRouterKey(entityType, value) {
window.Nuxeo = window.Nuxeo || {};
window.Nuxeo.UI = window.Nuxeo.UI || {};
window.Nuxeo.UI.config = window.Nuxeo.UI.config || {};
window.Nuxeo.UI.config.router = window.Nuxeo.UI.config.router || {};
window.Nuxeo.UI.config.router.key = window.Nuxeo.UI.config.router.key || {};
window.Nuxeo.UI.config.router.key = window.Nuxeo.UI.config.router.key || {};
window.Nuxeo.UI.config.router.key[entityType] = value;
}

suite('Nuxeo.RoutingBehavior', () => {
// define nuxeo-routed-element
customElements.define(
'nuxeo-routed-element',
class extends mixinBehaviors([RoutingBehavior], Nuxeo.Element) {
static get is() {
return 'nuxeo-routed-element';
}
},
);

setup(async () => {
await customElements.whenDefined('nuxeo-routed-element');
});

suite('urlFor', () => {
let el;
setup(async () => {
await customElements.whenDefined('nuxeo-routed-element');
// Mock router
RoutingBehavior.__router = {
document: (path, tab) => `${path.startsWith('/') ? 'path' : 'uid/'}${path}${tab ? `?p=${tab}` : ''}`,
};
sinon.spy(RoutingBehavior.__router, 'document');
el = await fixture(
html`
<nuxeo-routed-element></nuxeo-routed-element>
`,
);
});

test('should generate url for named route', async () => {
RoutingBehavior.__router.useHashbang = false;
RoutingBehavior.__router.baseUrl = '';
expect(el.urlFor('document', '/default-domain/workspaces/ws')).to.equal(`/path/default-domain/workspaces/ws`);
expect(RoutingBehavior.__router.document.calledOnce).to.be.true;
});

test('should generate url for named route when using hashbang', async () => {
RoutingBehavior.__router.useHashbang = true;
RoutingBehavior.__router.baseUrl = '';
expect(el.urlFor('document', '/default-domain/workspaces/ws')).to.equal(`/#!/path/default-domain/workspaces/ws`);
expect(RoutingBehavior.__router.document.calledOnce).to.be.true;
});

test('should generate url for named route when using base URL', async () => {
RoutingBehavior.__router.useHashbang = false;
RoutingBehavior.__router.baseUrl = 'base';
expect(el.urlFor('document', '/default-domain/workspaces/ws')).to.equal(`base/path/default-domain/workspaces/ws`);
expect(RoutingBehavior.__router.document.calledOnce).to.be.true;
});

test('should generate url for named route when using hashbang and base URL', async () => {
RoutingBehavior.__router.useHashbang = true;
RoutingBehavior.__router.baseUrl = 'base';
expect(el.urlFor('document', '/default-domain/workspaces/ws')).to.equal(
`base/#!/path/default-domain/workspaces/ws`,
);
expect(RoutingBehavior.__router.document.calledOnce).to.be.true;
});

test('should generate url for named route when passing a param', async () => {
RoutingBehavior.__router.useHashbang = false;
RoutingBehavior.__router.baseUrl = '';
expect(el.urlFor('document', '/default-domain/workspaces/ws', 'view')).to.equal(
`/path/default-domain/workspaces/ws?p=view`,
);
expect(RoutingBehavior.__router.document.calledOnce).to.be.true;
});

test('should generate url from object', async () => {
RoutingBehavior.__router.useHashbang = false;
RoutingBehavior.__router.baseUrl = '';
expect(el.urlFor({ 'entity-type': 'document', uid: 'abc123', path: '/default-domain/workspaces/ws' })).to.equal(
`/path/default-domain/workspaces/ws`,
);
expect(RoutingBehavior.__router.document.calledOnce).to.be.true;
});

test('should generate url from object when using hashbang', async () => {
RoutingBehavior.__router.useHashbang = true;
RoutingBehavior.__router.baseUrl = '';
expect(el.urlFor({ 'entity-type': 'document', uid: 'abc123', path: '/default-domain/workspaces/ws' })).to.equal(
`/#!/path/default-domain/workspaces/ws`,
);
expect(RoutingBehavior.__router.document.calledOnce).to.be.true;
});

test('should generate url from object when using base URL', async () => {
RoutingBehavior.__router.useHashbang = false;
RoutingBehavior.__router.baseUrl = 'base';
expect(el.urlFor({ 'entity-type': 'document', uid: 'abc123', path: '/default-domain/workspaces/ws' })).to.equal(
`base/path/default-domain/workspaces/ws`,
);
expect(RoutingBehavior.__router.document.calledOnce).to.be.true;
});

test('should generate url from object using hashbang and base URL', async () => {
RoutingBehavior.__router.useHashbang = true;
RoutingBehavior.__router.baseUrl = 'base';
expect(el.urlFor({ 'entity-type': 'document', uid: 'abc123', path: '/default-domain/workspaces/ws' })).to.equal(
`base/#!/path/default-domain/workspaces/ws`,
);
expect(RoutingBehavior.__router.document.calledOnce).to.be.true;
});

test('should generate url from object when passing a param', async () => {
RoutingBehavior.__router.useHashbang = false;
RoutingBehavior.__router.baseUrl = '';
expect(
el.urlFor({ 'entity-type': 'document', uid: 'abc123', path: '/default-domain/workspaces/ws' }, 'view'),
).to.equal(`/path/default-domain/workspaces/ws?p=view`);
expect(RoutingBehavior.__router.document.calledOnce).to.be.true;
});

test('should generate empty url from undefined route', async () => {
RoutingBehavior.__router.useHashbang = false;
RoutingBehavior.__router.baseUrl = '';
expect(el.urlFor()).to.equal('');
expect(RoutingBehavior.__router.document.notCalled).to.be.true;
});

suite('with route key for "document" entity-type set to "uid"', async () => {
setup(async () => {
setNuxeoRouterKey('document', 'uid');
});

teardown(async () => {
setNuxeoRouterKey('document'); // reset document key to undefined
});

test('should generate url from object', async () => {
RoutingBehavior.__router.useHashbang = false;
RoutingBehavior.__router.baseUrl = '';
expect(el.urlFor({ 'entity-type': 'document', uid: 'abc123', path: '/default-domain/workspaces/ws' })).to.equal(
`/uid/abc123`,
);
expect(RoutingBehavior.__router.document.calledOnce).to.be.true;
});
});
});
});
2 changes: 1 addition & 1 deletion ui/widgets/nuxeo-document-suggestion.js
Expand Up @@ -305,7 +305,7 @@ import { escapeHTML } from './nuxeo-selectivity.js';
if (typeof doc === 'string') {
return `<span>${escapeHTML(doc)}</span>`;
}
return `<a href="${this.urlFor('browse', doc.path)}">${escapeHTML(doc.title)}</a>`;
return `<a href="${this.urlFor(doc)}">${escapeHTML(doc.title)}</a>`;
}

_resultFormatter(doc) {
Expand Down