Skip to content

Commit

Permalink
Merge pull request #604 from namecheap/bugfix/NPF-2162-orderPos-lde
Browse files Browse the repository at this point in the history
fix: allow to override route's orderPos within LDE
  • Loading branch information
stas-nc authored Sep 10, 2024
2 parents 0dca47d + e4c555c commit 4012bac
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 147 deletions.
15 changes: 1 addition & 14 deletions ilc/server/tailor/merge-configs.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ module.exports = (original, override) => {
if (override.routes) {
override.routes.forEach((overrideRoute) => {
let indexOfOverrideRoute;
const beforeOf = overrideRoute.beforeOf;

const originalRoute = cloned.routes.find((route) => {
if (overrideRoute.routeId) {
return route.routeId === overrideRoute.routeId;
Expand All @@ -32,19 +30,8 @@ module.exports = (original, override) => {
cloned.routes.push(overrideRoute);
indexOfOverrideRoute = cloned.routes.length - 1;
}

if (beforeOf) {
const [routeThatShouldBeMoved] = cloned.routes.splice(indexOfOverrideRoute, 1);
const beforeRouteIndex = cloned.routes.findIndex(
(route) => beforeOf === route.route || beforeOf === route.routeId,
);
cloned.routes = [
...cloned.routes.slice(0, beforeRouteIndex),
routeThatShouldBeMoved,
...cloned.routes.slice(beforeRouteIndex),
];
}
});
cloned.routes = cloned.routes.sort((a, b) => a.orderPos - b.orderPos);
}

if (override.sharedLibs) {
Expand Down
141 changes: 12 additions & 129 deletions ilc/server/tailor/merge-configs.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,15 @@ describe('merge configs', () => {
routeId: 'commonRoute',
route: '*',
next: true,
orderPos: -99,
template: 'commonTemplate',
slots: {},
},
{
routeId: 'constRoute',
route: '/const',
next: false,
orderPos: 1,
slots: {
const: {
appName: apps['@portal/const'].name,
Expand All @@ -78,6 +80,7 @@ describe('merge configs', () => {
routeId: 'willChangeRoute',
route: '/will-change',
next: false,
orderPos: 99,
slots: {
willChange: {
appName: apps['@portal/will-change'].name,
Expand All @@ -97,6 +100,7 @@ describe('merge configs', () => {
routeId: 'errorsRoute',
route: '/404',
next: false,
orderPos: 50,
template: 'errorsTemplate',
slots: {},
},
Expand Down Expand Up @@ -171,6 +175,7 @@ describe('merge configs', () => {
routeId: 'newRoute',
route: '/new',
next: false,
orderPos: 90,
slots: {
new: {
appName: apps['@portal/const'].name,
Expand All @@ -180,6 +185,7 @@ describe('merge configs', () => {
{
routeId: 'willChangeRoute',
route: '/changed',
orderPos: -98,
slots: {
willChange: {
props: {
Expand Down Expand Up @@ -209,6 +215,7 @@ describe('merge configs', () => {
routeId: 'willChangeRoute',
route: '/changed',
next: false,
orderPos: -98,
slots: {
willChange: {
appName: apps['@portal/will-change'].name,
Expand Down Expand Up @@ -283,7 +290,7 @@ describe('merge configs', () => {

const mergedConfig = {
...registryConfig,
routes: [commonRoute, constRoute, mergedRoute, newRoute],
routes: [commonRoute, mergedRoute, constRoute, newRoute],
};

chai.expect(mergeConfigs(registryConfig, overrideConfig)).to.be.eql(mergedConfig);
Expand Down Expand Up @@ -319,7 +326,7 @@ describe('merge configs', () => {
'@portal/will-change': mergedApp,
'@portal/new': overrideConfig.apps['@portal/new'],
},
routes: [commonRoute, constRoute, mergedRoute, newRoute],
routes: [commonRoute, mergedRoute, constRoute, newRoute],
sharedLibs: mergedSharedLibs,
};

Expand All @@ -335,6 +342,7 @@ describe('merge configs', () => {
routeId: 'shouldBeChangedByRouteProperty',
route: '/should-be-changed-by-route-property',
next: false,
orderPos: 10,
slots: {
const: {
appName: apps['@portal/const'].name,
Expand All @@ -355,6 +363,7 @@ describe('merge configs', () => {
{
route: '/should-be-changed-by-route-property',
next: true,
orderPos: 100,
slots: {
new: {
appName: apps['@portal/const'].name,
Expand All @@ -372,6 +381,7 @@ describe('merge configs', () => {
routeId: 'shouldBeChangedByRouteProperty',
route: '/should-be-changed-by-route-property',
next: true,
orderPos: 100,
slots: {
const: {
appName: apps['@portal/const'].name,
Expand All @@ -390,132 +400,5 @@ describe('merge configs', () => {
],
});
});

it('should put a new route before of another route', () => {
const newRoute = {
route: '/new-one',
next: false,
beforeOf: '/before-of-this-route-should-be-new-one',
slots: {
const: {
appName: apps['@portal/const'].name,
props: {
constSlotFirstProp: 'constSlotFirstProp',
constSlotSecondProp: {
firstPropOfConstSlotSecondProp: 'firstPropOfConstSlotSecondProp',
},
},
},
},
};

const beforeOfThisRouteShouldBeNewOne = {
routeId: 'beforeOfThisRouteShouldBeNewOne',
route: '/before-of-this-route-should-be-new-one',
next: false,
slots: {
const: {
appName: apps['@portal/const'].name,
props: {
constSlotFirstProp: 'constSlotFirstProp',
constSlotSecondProp: {
firstPropOfConstSlotSecondProp: 'firstPropOfConstSlotSecondProp',
},
},
},
},
};

const registryConfigWithExtraRouteToTest = {
...registryConfig,
routes: [...registryConfig.routes, beforeOfThisRouteShouldBeNewOne],
};

const overrideConfig = {
routes: [newRoute],
};

chai.expect(mergeConfigs(registryConfigWithExtraRouteToTest, overrideConfig)).to.be.eql({
...registryConfig,
routes: [...registryConfig.routes, newRoute, beforeOfThisRouteShouldBeNewOne],
});
});

it('should override a route and move it before of another route', () => {
const beforeOfThisRouteShouldBeNewOne = {
routeId: 'beforeOfThisRouteShouldBeNewOne',
route: '/before-of-this-route-should-be-new-one',
next: false,
slots: {},
};

const registryConfigWithExtraRouteToTest = {
...registryConfig,
routes: [
...registryConfig.routes,
beforeOfThisRouteShouldBeNewOne,
{
routeId: 'shouldBeChangedAndMovedBeforeOfAnotherRoute',
route: '/should-be-changed-and-moved-before-of-another-route',
next: false,
slots: {
const: {
appName: apps['@portal/const'].name,
props: {
constSlotFirstProp: 'constSlotFirstProp',
constSlotSecondProp: {
firstPropOfConstSlotSecondProp: 'firstPropOfConstSlotSecondProp',
},
},
},
},
},
],
};

const overrideConfig = {
routes: [
{
routeId: 'shouldBeChangedAndMovedBeforeOfAnotherRoute',
route: '/should-be-changed-and-moved-before-of-another-route',
beforeOf: '/before-of-this-route-should-be-new-one',
next: false,
slots: {
new: {
appName: apps['@portal/const'].name,
},
},
},
],
};

chai.expect(mergeConfigs(registryConfigWithExtraRouteToTest, overrideConfig)).to.be.eql({
...registryConfig,
routes: [
...registryConfig.routes,
{
routeId: 'shouldBeChangedAndMovedBeforeOfAnotherRoute',
route: '/should-be-changed-and-moved-before-of-another-route',
beforeOf: '/before-of-this-route-should-be-new-one',
next: false,
slots: {
const: {
appName: apps['@portal/const'].name,
props: {
constSlotFirstProp: 'constSlotFirstProp',
constSlotSecondProp: {
firstPropOfConstSlotSecondProp: 'firstPropOfConstSlotSecondProp',
},
},
},
new: {
appName: apps['@portal/const'].name,
},
},
},
beforeOfThisRouteShouldBeNewOne,
],
});
});
});
});
11 changes: 10 additions & 1 deletion registry/server/routes/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,16 @@ router.get('/', async (req, res) => {
meta: {},
},
_.omitBy(
_.pick(routeItem, ['routeId', 'route', 'next', 'template', 'specialRole', 'domain', 'versionId']),
_.pick(routeItem, [
'routeId',
'route',
'next',
'template',
'specialRole',
'domain',
'orderPos',
'versionId',
]),
_.isNull,
),
);
Expand Down
7 changes: 4 additions & 3 deletions registry/tests/config.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,17 +131,18 @@ describe('Tests /api/v1/config', () => {

expect(response.body.routes).to.deep.include({
routeId,
..._.pick(example.appRoutes, ['route', 'next', 'slots', 'meta']),
..._.pick(example.appRoutes, ['route', 'next', 'slots', 'meta', 'orderPos']),
});

expect(response.body.routes).to.deep.include({
routeId: routeIdWithDomain,
..._.pick(example.appRoutes, ['route', 'next', 'slots', 'meta']),
domain: example.routerDomains.domainName,
orderPos: 123,
..._.pick(example.appRoutes, ['route', 'next', 'slots', 'meta']),
});

expect(response.body.routes).to.deep.include({
..._.omit(example.appRoutesWithoutSlots, ['orderPos', 'templateName']),
..._.omit(example.appRoutesWithoutSlots, ['templateName']),
template: example.appRoutesWithoutSlots.templateName,
});

Expand Down

0 comments on commit 4012bac

Please sign in to comment.