Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: added ability to conveniently get hostname from client/server API of the Transition hooks plugin #305

Merged
merged 1 commit into from
May 31, 2021
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions ilc/client/GuardManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,12 @@ export default class GuardManager {
route: {
meta: route.meta,
url: route.reqUrl,
hostname: window.location.hostname,
Copy link
Contributor

Choose a reason for hiding this comment

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

it's minor but I think it's a little bit better to use host instead of hostname. what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope :) http://bl.ocks.org/abernier/3070589 I would work inconsistently with SSR then.

},
prevRoute: {
meta: prevRoute.meta,
url: prevRoute.reqUrl,
hostname: window.location.hostname,
},
navigate: this.#router.navigateToUrl,
});
Expand Down
16 changes: 8 additions & 8 deletions ilc/client/GuardManager.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ describe('GuardManager', () => {

for (const hook of hooks) {
sinon.assert.calledOnceWithExactly(hook, {
route: {meta: route.meta, url: route.reqUrl},
prevRoute: {meta: prevRoute.meta, url: prevRoute.reqUrl},
route: {meta: route.meta, url: route.reqUrl, hostname: window.location.hostname},
prevRoute: {meta: prevRoute.meta, url: prevRoute.reqUrl, hostname: window.location.hostname},
navigate: router.navigateToUrl
});
}
Expand Down Expand Up @@ -129,8 +129,8 @@ describe('GuardManager', () => {

for (const hook of [hooks[0], hooks[1]]) {
sinon.assert.calledOnceWithExactly(hook, {
route: {meta: route.meta, url: route.reqUrl},
prevRoute: {meta: prevRoute.meta, url: prevRoute.reqUrl},
route: {meta: route.meta, url: route.reqUrl, hostname: window.location.hostname},
prevRoute: {meta: prevRoute.meta, url: prevRoute.reqUrl, hostname: window.location.hostname},
navigate: router.navigateToUrl
});
}
Expand Down Expand Up @@ -159,8 +159,8 @@ describe('GuardManager', () => {

for (const hook of [hooks[0], hooks[1]]) {
sinon.assert.calledOnceWithExactly(hook, {
route: {meta: route.meta, url: route.reqUrl},
prevRoute: {meta: prevRoute.meta, url: prevRoute.reqUrl},
route: {meta: route.meta, url: route.reqUrl, hostname: window.location.hostname},
prevRoute: {meta: prevRoute.meta, url: prevRoute.reqUrl, hostname: window.location.hostname},
navigate: router.navigateToUrl
});
}
Expand Down Expand Up @@ -189,8 +189,8 @@ describe('GuardManager', () => {

for (const hook of [hooks[0], hooks[1]]) {
sinon.assert.calledOnceWithExactly(hook, {
route: {meta: route.meta, url: route.reqUrl},
prevRoute: {meta: prevRoute.meta, url: prevRoute.reqUrl},
route: {meta: route.meta, url: route.reqUrl, hostname: window.location.hostname},
prevRoute: {meta: prevRoute.meta, url: prevRoute.reqUrl, hostname: window.location.hostname},
navigate: router.navigateToUrl
});
}
Expand Down
13 changes: 9 additions & 4 deletions ilc/server/GuardManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,12 @@ class GuardManager {
this.#transitionHooksPlugin = pluginManager.getTransitionHooksPlugin();
}

async redirectTo(log, req) {
const route = req.router.getRoute();
/**
* @param {FastifyRequest} req
* @return {Promise<null|*>}
*/
async redirectTo(req) {
const route = req.raw.router.getRoute();

if (route.specialRole !== null) {
return null;
Expand All @@ -27,9 +31,10 @@ class GuardManager {
route: {
meta: route.meta,
url: route.reqUrl,
hostname: req.hostname,
},
log,
req,
log: req.log,
req: req.raw,
});

if (action.type === actionTypes.redirect && action.newLocation) {
Expand Down
31 changes: 18 additions & 13 deletions ilc/server/GuardManager.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,18 +99,23 @@ describe('GuardManager', () => {
debug: () => { },
trace: () => { },
});
const req = Object.freeze({
const rawReq = Object.freeze({
router: {
getRoute: () => route,
},
});
const req = {
raw: rawReq,
log,
hostname: 'test.com'
};

describe('should have access to a provided URL', () => {
it('if transition hooks are non existent', async () => {
pluginManager.getTransitionHooksPlugin.returns(transitionHooksPlugin);
transitionHooksPlugin.getTransitionHooks.returns([]);

const redirectTo = await new GuardManager(pluginManager).redirectTo(log, req);
const redirectTo = await new GuardManager(pluginManager).redirectTo(req);

chai.expect(redirectTo).to.be.null;
});
Expand All @@ -122,12 +127,12 @@ describe('GuardManager', () => {
const route = Object.freeze({
specialRole: 404,
});
const req = Object.freeze({
const rawReq = Object.freeze({
router: {
getRoute: () => route,
},
});
const redirectTo = await new GuardManager(pluginManager).redirectTo(log, req);
const redirectTo = await new GuardManager(pluginManager).redirectTo({...req, raw: rawReq});

chai.expect(redirectTo).to.be.null;
});
Expand All @@ -141,12 +146,12 @@ describe('GuardManager', () => {
pluginManager.getTransitionHooksPlugin.returns(transitionHooksPlugin);
transitionHooksPlugin.getTransitionHooks.returns(hooks);

const redirectTo = await new GuardManager(pluginManager).redirectTo(log, req);
const redirectTo = await new GuardManager(pluginManager).redirectTo(req);

for (const hook of hooks) {
chai.expect(hook.calledOnceWith({
route: {meta: route.meta, url: route.reqUrl},
req,
route: {meta: route.meta, url: route.reqUrl, hostname: req.hostname},
req: rawReq,
log,
})).to.be.true;
}
Expand All @@ -168,7 +173,7 @@ describe('GuardManager', () => {
pluginManager.getTransitionHooksPlugin.returns(transitionHooksPlugin);
transitionHooksPlugin.getTransitionHooks.returns(hooks);

await chai.expect(new GuardManager(pluginManager).redirectTo(log, req)).to.eventually.be.rejected.then((rejectedError) => {
await chai.expect(new GuardManager(pluginManager).redirectTo(req)).to.eventually.be.rejected.then((rejectedError) => {
chai.expect(rejectedError).to.be.instanceOf(errors.GuardTransitionHookError);
chai.expect(rejectedError.data).to.be.eql({
hookIndex: 1,
Expand All @@ -178,8 +183,8 @@ describe('GuardManager', () => {

for (const hook of [hooks[0], hooks[1]]) {
chai.expect(hook.calledOnceWith({
route: {meta: route.meta, url: route.reqUrl},
req,
route: {meta: route.meta, url: route.reqUrl, hostname: req.hostname},
req: rawReq,
log,
})).to.be.true;
}
Expand All @@ -201,12 +206,12 @@ describe('GuardManager', () => {
pluginManager.getTransitionHooksPlugin.returns(transitionHooksPlugin);
transitionHooksPlugin.getTransitionHooks.returns(hooks);

const redirectTo = await new GuardManager(pluginManager).redirectTo(log, req);
const redirectTo = await new GuardManager(pluginManager).redirectTo(req);

for (const hook of [hooks[0], hooks[1]]) {
chai.expect(hook.calledOnceWith({
route: {meta: route.meta, url: route.reqUrl},
req,
route: {meta: route.meta, url: route.reqUrl, hostname: req.hostname},
req: rawReq,
log,
})).to.be.true;
}
Expand Down
2 changes: 1 addition & 1 deletion ilc/server/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ module.exports = (registryService, pluginManager) => {
req.raw.registryConfig = registryConfig;
req.raw.router = new ServerRouter(req.log, req.raw, unlocalizedUrl);

const redirectTo = await guardManager.redirectTo(req.log, req.raw);
const redirectTo = await guardManager.redirectTo(req);

if (redirectTo) {
res.redirect(urlProcessor.process(i18n.localizeUrl(registryConfig.settings.i18n, redirectTo, {
Expand Down