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

404 support for non existing paths #162

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
875676f
Detect the moment when popup should appear
parostatkiem-zz Oct 24, 2018
477e8f2
Implement not-exact-path alert
parostatkiem-zz Oct 24, 2018
e4bd18e
Handle 404 when route is absolutely not found
parostatkiem-zz Oct 24, 2018
f395863
Add min-width to alert
parostatkiem-zz Oct 24, 2018
64414ee
Rewrite the mechanizm
parostatkiem-zz Oct 24, 2018
b80b7d8
Merge branch 'master' of https://github.com/kyma-project/luigi into 4…
parostatkiem-zz Oct 25, 2018
b75a50c
Change CustomEvents to postMessage
parostatkiem-zz Oct 25, 2018
d694329
Separate tests into directories
parostatkiem-zz Oct 25, 2018
1385ab4
Test the containsAllSegments function
parostatkiem-zz Oct 25, 2018
8389703
Remove obsolete console.warn()
parostatkiem-zz Oct 25, 2018
db71aa4
Rename error, rename variable in alert event, add some 'anti-undefine…
parostatkiem-zz Oct 26, 2018
a6fef26
Correct wrong errorchecking in containsAllSegments, add another test
parostatkiem-zz Oct 26, 2018
78661a8
Merge branch 'master' of https://github.com/kyma-project/luigi into 4…
parostatkiem-zz Oct 26, 2018
e3c1499
Add wrong routes example using linkManager
parostatkiem-zz Oct 31, 2018
af7a078
Add one UI test
parostatkiem-zz Oct 31, 2018
e7c12f6
Merge branch 'master' of https://github.com/kyma-project/luigi into 4…
parostatkiem-zz Oct 31, 2018
1b149dd
Fix bugs
parostatkiem-zz Oct 31, 2018
b311a62
Add UI tests
parostatkiem-zz Oct 31, 2018
56ad343
Merge branch 'master' of https://github.com/kyma-project/luigi into 4…
parostatkiem-zz Nov 5, 2018
a297a41
Merge branch 'master' of https://github.com/kyma-project/luigi into 4…
parostatkiem-zz Nov 7, 2018
bd0f97b
Merge branch 'master' into 404-support-for-non-existing-paths
kwiatekus Nov 7, 2018
c3c9f7a
Remove unnecessary line breaks
parostatkiem-zz Nov 7, 2018
149be2c
Add additional length check before forEach loop
parostatkiem-zz Nov 7, 2018
eedf9bb
Show true url when original url was partly bad & fix tests
parostatkiem-zz Nov 9, 2018
51381a0
Replace pushState with replaceState
parostatkiem-zz Nov 9, 2018
42bc64c
Add support for non-path routing
parostatkiem-zz Nov 9, 2018
c1c3d09
Merge branch 'master' of github.com:kyma-project/luigi into 404-suppo…
kwiatekus Nov 9, 2018
0b73f82
Merge branch 'master' into 404-support-for-non-existing-paths
parostatkiem Nov 9, 2018
4abb343
Merge branch 'master' of https://github.com/kyma-project/luigi into 4…
parostatkiem-zz Nov 13, 2018
e9798e2
Add links to alerts & redirect to '/' after 404 error
parostatkiem-zz Nov 13, 2018
d35f53c
Fix & improve tets
parostatkiem-zz Nov 13, 2018
f7313fd
Merge branch 'master' of https://github.com/kyma-project/luigi into 4…
parostatkiem-zz Nov 14, 2018
ad31069
aa
parostatkiem-zz Nov 14, 2018
67adae6
Change way of handling alert from postMessage to component's state
parostatkiem-zz Nov 14, 2018
6d17ee0
Fix tests
parostatkiem-zz Nov 14, 2018
6f585b5
Remove obsolete line
parostatkiem-zz Nov 14, 2018
e54a288
Merge branch 'master' into 404-support-for-non-existing-paths
jesusreal Nov 14, 2018
306d852
Remove duplicating tests & remove alert link
parostatkiem-zz Nov 16, 2018
a7ccb73
Merge branch 'master' of https://github.com/kyma-project/luigi into 4…
parostatkiem-zz Nov 16, 2018
88d3e2a
Ignore GET parameters when detecting wrong URL and test it
parostatkiem-zz Nov 16, 2018
8008146
Shorten variable name & syntax
parostatkiem-zz Nov 16, 2018
7092f03
Simplify containsAllSegments
parostatkiem-zz Nov 19, 2018
24f7251
Merge branch 'master' of https://github.com/kyma-project/luigi into 4…
parostatkiem-zz Nov 19, 2018
2e47937
Merge branch 'master' of https://github.com/kyma-project/luigi into 4…
parostatkiem-zz Nov 19, 2018
d489a59
Merge branch 'master' into 404-support-for-non-existing-paths
kwiatekus Nov 20, 2018
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
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,39 @@ describe('Luigi client features', () => {
});
});

describe('linkManager wrong paths navigation', () => {
let $iframeBody;
beforeEach(() => {
cy.get('iframe').then($iframe => {
$iframeBody = $iframe.contents().find('body');
cy.goToFeaturesPage($iframeBody);
});
});
it('navigate to a partly wrong link', () => {
cy.wrap($iframeBody)
.contains('Partly wrong link')
.click();
cy.location().should(loc => {
expect(loc.hash).to.eq('#/projects/pr2/miscellaneous2');
});
cy.get('.fd-alert').contains(
'Could not map the exact target node for the requested route projects/pr2/miscellaneous2/maskopatol'
);
});

it('navigate to a totally wrong link', () => {
cy.wrap($iframeBody)
.contains('Totally wrong link')
.click();
cy.location().should(loc => {
expect(loc.hash).to.eq('#/overview');
});
cy.get('.fd-alert').contains(
'Could not find the requested route maskopatol/has/a/child'
);
});
});

describe('uxManager', () => {
it('backdrop', () => {
cy.wait(500);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,76 +1,99 @@
<section class="fd-section">
<div class="fd-section__header">
<h1 class="fd-section__title">Project {{ projectId }}</h1>
</div>
<div class="fd-panel">
<p><strong>LuigiClient uxManager methods:</strong></p>
<button class="fd-button" (click)="toggleModal()">Add backdrop</button>
<p>
<app-code-snippet data="luigiClient.uxManager().addBackdrop()"></app-code-snippet>
<app-code-snippet data="luigiClient.uxManager().removeBackdrop()"></app-code-snippet>
</p>
</div>
<div class="fd-section__header">
<h1 class="fd-section__title">Project {{ projectId }}</h1>
</div>
<div class="fd-panel">
<p><strong>LuigiClient uxManager methods:</strong></p>
<button class="fd-button" (click)="toggleModal()">Add backdrop</button>
<p>
<app-code-snippet data="luigiClient.uxManager().addBackdrop()"></app-code-snippet>
<app-code-snippet data="luigiClient.uxManager().removeBackdrop()"></app-code-snippet>
</p>
</div>
</section>

<section class="fd-section" *ngIf="preservedViewCallbackContext">
<div class="fd-panel">
<div class="fd-alert" role="alert">
<span class="fd-status-label fd-status-label--available"></span> Context received from linkManager().goBack():<br />
<pre>{{ preservedViewCallbackContext | json }}</pre>
</div>
</div>
<div class="fd-panel">
<div class="fd-alert" role="alert">
<span class="fd-status-label fd-status-label--available"></span> Context received from linkManager().goBack():<br />
<pre>{{ preservedViewCallbackContext | json }}</pre>
</div>
</div>
</section>

<section class="fd-section link-manager" *ngIf="projectId && projectId !== 'overview'">
<div class="fd-panel">
<p><strong>LuigiClient linkManager methods:</strong></p>
<ul class="fd-list-group">
<li class="fd-list-group__item">
<a href="javascript:void(0)" (click)="luigiClient.linkManager().navigate('/overview')">absolute: to overview</a>
<app-code-snippet data="luigiClient.linkManager().navigate('/overview')"></app-code-snippet>
</li>
<li class="fd-list-group__item">
<a href="javascript:void(0)" (click)="luigiClient.linkManager().navigate('users/groups/stakeholders')">relative: to stakeholders</a>
<app-code-snippet data="luigiClient.linkManager().navigate('users/groups/stakeholders')"></app-code-snippet>
</li>
<li class="fd-list-group__item">
<a href="javascript:void(0)" (click)="luigiClient.linkManager().fromClosestContext().navigate('/users/groups/stakeholders')">closest parent: to stakeholders</a>
<app-code-snippet data="luigiClient.linkManager().fromClosestContext().navigate('/users/groups/stakeholders')"></app-code-snippet>
</li>
<li class="fd-list-group__item">
<a href="javascript:void(0)" (click)="luigiClient.linkManager().fromContext('project').navigate('/settings')">parent by name: project to settings</a>
<app-code-snippet data="luigiClient.linkManager().fromContext('project').navigate('/settings')"></app-code-snippet>
</li>
<li class="fd-list-group__item">
<a href="javascript:void(0)" (click)="luigiClient.linkManager().fromClosestContext().withParams({foo: 'bar'}).navigate('settings')">project to settings with params (foo=bar)</a>
<app-code-snippet data="luigiClient.linkManager().fromClosestContext().withParams({foo: 'bar'}).navigate('settings')"></app-code-snippet>
</li>
<li class="fd-list-group__item">
<a href="javascript:void(0)" (click)="luigiClient.linkManager().fromContext('FOOMARK').navigate('/settings')">parent by name: with nonexisting context</a> (look at the console)
<app-code-snippet data="luigiClient.linkManager().fromContext('FOOMARK').navigate('/settings')"></app-code-snippet>
</li>
<li class="fd-list-group__item">
<a href="javascript:void(0)" (click)="luigiClient.linkManager().navigate('/settings', null, true)">with preserved view: project to global settings and back</a>
<app-code-snippet data="luigiClient.linkManager().navigate('/settings', null, true)"></app-code-snippet>
</li>
<li class="fd-list-group__item check-path">
<span>Check if path exists</span>
<app-code-snippet data="luigiClient.linkManager().pathExists('{{pathExists.formValue}}')"></app-code-snippet>
<span>
<input type="text" [(ngModel)]="pathExists.formValue" (input)="resetPathExistsResult()"/>
<button class="fd-button" (click)="checkIfPathExists()">Check</button>
</span>
<p class="check-path-result">
<ng-container *ngIf="pathExists.result === true">
Path {{pathExists.formValue}} exists!
</ng-container>
<ng-container *ngIf="pathExists.result === false">
Path {{pathExists.formValue}} does not exist!
</ng-container>
</p>
</li>
</ul>
</div>
<div class="fd-panel">
<p><strong>LuigiClient linkManager methods:</strong></p>
<ul class="fd-list-group">
<li class="fd-list-group__item">
<a href="javascript:void(0)" (click)="luigiClient.linkManager().navigate('/overview')">absolute: to overview</a>
<app-code-snippet data="luigiClient.linkManager().navigate('/overview')"></app-code-snippet>
</li>
<li class="fd-list-group__item">
<a href="javascript:void(0)" (click)="luigiClient.linkManager().navigate('users/groups/stakeholders')">relative: to stakeholders</a>
<app-code-snippet data="luigiClient.linkManager().navigate('users/groups/stakeholders')"></app-code-snippet>
</li>
<li class="fd-list-group__item">
<a href="javascript:void(0)" (click)="luigiClient.linkManager().fromClosestContext().navigate('/users/groups/stakeholders')">
closest parent: to stakeholders</a>
<app-code-snippet data="luigiClient.linkManager().fromClosestContext().navigate('/users/groups/stakeholders')"></app-code-snippet>
</li>
<li class="fd-list-group__item">
<a href="javascript:void(0)" (click)="luigiClient.linkManager().fromContext('project').navigate('/settings')">
parent by name: project to settings</a>
<app-code-snippet data="luigiClient.linkManager().fromContext('project').navigate('/settings')"></app-code-snippet>
</li>
<li class="fd-list-group__item">
<a href="javascript:void(0)" (click)="luigiClient.linkManager().fromClosestContext().withParams({foo: 'bar'}).navigate('settings')">
project to settings with params (foo=bar)</a>
<app-code-snippet data="luigiClient.linkManager().fromClosestContext().withParams({foo: 'bar'}).navigate('settings')"></app-code-snippet>
</li>
<li class="fd-list-group__item">
<a href="javascript:void(0)" (click)="luigiClient.linkManager().fromContext('FOOMARK').navigate('/settings')">
parent by name: with nonexisting context</a> (look at the console)
<app-code-snippet data="luigiClient.linkManager().fromContext('FOOMARK').navigate('/settings')"></app-code-snippet>
</li>
<li class="fd-list-group__item">
<a href="javascript:void(0)" (click)="luigiClient.linkManager().navigate('/settings', null, true)">
with preserved view: project to global settings and back</a>
<app-code-snippet data="luigiClient.linkManager().navigate('/settings', null, true)"></app-code-snippet>
</li>
<li class="fd-list-group__item check-path">
<span>Check if path exists</span>
<app-code-snippet data="luigiClient.linkManager().pathExists('{{pathExists.formValue}}')"></app-code-snippet>
<span>
<input type="text" [(ngModel)]="pathExists.formValue" (input)="resetPathExistsResult()" />
<button class="fd-button" (click)="checkIfPathExists()">Check</button>
</span>
<p class="check-path-result">
<ng-container *ngIf="pathExists.result === true">
Path {{pathExists.formValue}} exists!
</ng-container>
<ng-container *ngIf="pathExists.result === false">
Path {{pathExists.formValue}} does not exist!
</ng-container>
</p>
</li>
</ul>
</div>
</section>

<app-modal [modalActive]="modalActive" (modalClosed)="toggleModal()"></app-modal>
<section class="fd-section" *ngIf="projectId && projectId !== 'overview'">
<div class="fd-panel">
<p><strong>LuigiClient - wrong paths in linkManager.navigate():</strong></p>
<ul class="fd-list-group">
<li class="fd-list-group__item">
<a href="javascript:void(0)" (click)="luigiClient.linkManager().navigate('/projects/pr2/miscellaneous2/maskopatol')">Partly
wrong link
</a>
<app-code-snippet data="luigiClient.linkManager().navigate('/projects/pr2/miscellaneous2/maskopatol')"></app-code-snippet>
</li>
<li class="fd-list-group__item">
<a href="javascript:void(0)" (click)="luigiClient.linkManager().navigate('/maskopatol/has/a/child')">Totally wrong link</a>
<app-code-snippet data="luigiClient.linkManager().navigate('/maskopatol/has/a/child')"></app-code-snippet>
</li>
</ul>
</div>
</section>

<app-modal [modalActive]="modalActive" (modalClosed)="toggleModal()"></app-modal>
2 changes: 1 addition & 1 deletion core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
"scripts": {
"bundle": "webpack --display-error-details",
"bundle-develop": "webpack -d --watch",
"test": "./node_modules/mocha/bin/mocha --require babel-register --require babel-polyfill --require jsdom-global/register",
"test": "./node_modules/mocha/bin/mocha --require babel-register --require babel-polyfill --require jsdom-global/register --recursive test",
"bundlesize": "bundlesize",
"prepush": "npm test && npm run bundle && npm run bundlesize"
},
Expand Down
33 changes: 28 additions & 5 deletions core/src/App.html
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
<div id="app {hideNav? 'no-nav' : ''}">
{#if alert && alert.message}
<div class="fd-alert fd-alert--error fd-alert--dismissible" role="alert" id="j2ALl423">
<button on:click="set({alert:null})" class="fd-alert__close" aria-controls="j2ALl423" aria-label="Close"></button>
{alert.message} {alert.link}
</div>
{/if}
<Backdrop>
<div class="fd-page iframeContainer" use:init="context"></div>
</Backdrop>
Expand Down Expand Up @@ -122,12 +128,14 @@
};

const handleNavigation = async (component, data, config) => {
const path = buildPath(component, data.params);
const matchedPath = await Routing.matchPath(path);
if (matchedPath !== null) {
addPreserveView(component, data, config);
Routing.navigateTo(matchedPath);
let path = buildPath(component, data.params);

if (path[0] !== '/') {
path = '/' + path; //add leading slash if necessary
}

addPreserveView(component, data, config);
Routing.navigateTo(path); //navigate to the raw path. Any errors/alerts are handled later
};

const sendContextToClient = (component, config, goBackContext = {}) => {
Expand Down Expand Up @@ -296,6 +304,7 @@
<style type="text/scss">
@import 'node_modules/fundamental-ui/scss/layout/page';
@import 'node_modules/fundamental-ui/scss/components/spinner';
@import 'node_modules/fundamental-ui/scss/components/alert';
@import 'static.css';

:global(html) {
Expand Down Expand Up @@ -369,4 +378,18 @@
right: 0;
}
}

$topNavHeight: 50px;
$leftNavWidth: 320px;
.fd-alert {
position: absolute;
min-width: 20rem;
width: calc(100% - 2 * (#{$leftNavWidth} + 1rem));
top: calc(#{$topNavHeight} + 1rem);
left: calc(#{$leftNavWidth} + 1rem);
z-index: 2;
.fd-alert__close {
cursor: pointer;
}
}
</style>
26 changes: 24 additions & 2 deletions core/src/services/routing.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { LuigiConfig } from './config';
import {
getPathWithoutHash,
getUrlWithoutHash,
containsAllSegments,
isIE,
getConfigValueFromObject
} from '../utilities/helpers';
Expand Down Expand Up @@ -308,10 +309,31 @@ export const handleRouteChange = async (path, component, node, config) => {
if (routeExists) {
const defaultChildNode = getDefaultChildNode(pathData);
navigateTo(`${pathUrl ? `/${pathUrl}` : ''}/${defaultChildNode}`);
} // TODO else display 404 page
} else {
const alert = {
message: 'Could not find the requested route',
link: pathUrl
};

component.set({ alert });
navigateTo('/');
//error 404
}
return;
}

if (!containsAllSegments(pathUrl, pathData.navigationPath)) {
const matchedPath = await matchPath(pathUrl);

const alert = {
message: 'Could not map the exact target node for the requested route',
link: pathUrl
};

component.set({ alert });
navigateTo(matchedPath);
}

const previousCompData = component.get();
component.set({
hideNav,
Expand Down Expand Up @@ -392,7 +414,7 @@ export const matchPath = async path => {
@param windowElem object defaults to window
@param documentElem object defaults to document
*/
export const navigateTo = (route, windowElem = window) => {
export const navigateTo = async (route, windowElem = window) => {
if (LuigiConfig.getConfigValue('routing.useHashRouting')) {
windowElem.location.hash = route;
return;
Expand Down
23 changes: 23 additions & 0 deletions core/src/utilities/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,29 @@ export const prependOrigin = path => {
return window.location.origin;
};

export const containsAllSegments = (sourceUrl, targetPathSegments) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

There is still a bug when the user introduces a url with a trailing slash like '/projects/pr2/' (sourceUrl param in function)
Furthermore the implementation could be simpler. My proposal is the following (unit tests are still green after refactoring)

if (!sourceUrl || !targetPathSegments || !targetPathSegments.length) {
console.error(
'Ooops, seems like the developers have misconfigured something'
);
return false;
}

const pathSegmentsUrl = targetPathSegments
.slice(1)
.map(x => x.pathSegment)
.join('/');
const mandatorySegmentsUrl = removeTrailingSlash(sourceUrl.split('?')[0]);
return pathSegmentsUrl === mandatorySegmentsUrl;
};

/**
* Prepend current url to redirect_uri, if it is a relative path
* @param {str} string from which any number of trailing slashes should be removed
* @returns string string without any trailing slash
*/
export const removeTrailingSlash = str => str.replace(/\/+$/, '');

/*
* Gets value of the given property on the given object.
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const chai = require('chai');
const assert = chai.assert;
import { LuigiConfig } from '../src/services/config';
import { LuigiConfig } from '../../src/services/config';

describe('Config', () => {
describe('getConfigBooleanValue()', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
const navigation = require('../src/navigation/services/navigation');
const navigation = require('../../src/navigation/services/navigation');
const chai = require('chai');
const expect = chai.expect;
const assert = chai.assert;
const sinon = require('sinon');
import { LuigiConfig } from '../src/services/config';
import { LuigiConfig } from '../../src/services/config';

const sampleNavPromise = new Promise(function(resolve) {
const lazyLoadedChildrenNodesProviderFn = () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ const expect = chai.expect;
const assert = chai.assert;
const sinon = require('sinon');
const MockBrowser = require('mock-browser').mocks.MockBrowser;
const routing = require('../src/services/routing');
import { deepMerge } from '../src/utilities/helpers.js';
const routing = require('../../src/services/routing');
import { deepMerge } from '../../src/utilities/helpers.js';
import { afterEach } from 'mocha';
import { LuigiConfig } from '../src/services/config';
import { LuigiConfig } from '../../src/services/config';

describe('Routing', () => {
let component;
Expand Down Expand Up @@ -742,7 +742,7 @@ describe('Routing', () => {
});

describe('defaultChildNodes', () => {
const routing = rewire('../src/services/routing');
const routing = rewire('../../src/services/routing');
const getDefaultChildNode = routing.__get__('getDefaultChildNode');
const getPathData = function() {
return {
Expand Down
Loading