Skip to content

Commit e3dad0f

Browse files
authored
Redirect user to active cluster when there is no cluster selected (#530)
* Redirect user to active cluster when there is no cluster selected * fix linting issue on scenario file
1 parent f64c457 commit e3dad0f

File tree

5 files changed

+162
-8
lines changed

5 files changed

+162
-8
lines changed

client/containers/domain/connector.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,12 @@
2020
// THE SOFTWARE.
2121

2222
import { connect } from 'vuex-connect';
23+
import { ROUTE_REPLACE } from '../route/action-types';
2324
import {
2425
ROUTE_PARAMS_CLUSTER_NAME,
2526
ROUTE_PARAMS_DOMAIN,
2627
} from '../route/getter-types';
28+
import { ACTIVE_STATUS_CLUSTER } from '../active-status/getter-types';
2729
import {
2830
DOMAIN_FETCH,
2931
DOMAIN_ON_MOUNT,
@@ -44,6 +46,7 @@ const actionsToEvents = {
4446

4547
const gettersToProps = {
4648
clusterName: ROUTE_PARAMS_CLUSTER_NAME,
49+
activeCluster: ACTIVE_STATUS_CLUSTER,
4750
domainName: ROUTE_PARAMS_DOMAIN,
4851
error: DOMAIN_ERROR,
4952
isLoading: DOMAIN_IS_LOADING,
@@ -53,6 +56,20 @@ const gettersToProps = {
5356

5457
const lifecycle = {
5558
mounted: ({ dispatch }) => dispatch(DOMAIN_ON_MOUNT),
59+
updated({ dispatch, state }) {
60+
const urlParamClusterName = this.clusterName;
61+
62+
if (!urlParamClusterName && this.activeCluster?.clusterName) {
63+
// in some cases users have urls that are generic (with no cluster name specified) those are used when we want to auto redirect the user to one of the clusters by default without carring about which cluster.
64+
dispatch(ROUTE_REPLACE, {
65+
name: state.route.name,
66+
params: {
67+
...state.route.params,
68+
clusterName: this.activeCluster?.clusterName,
69+
},
70+
});
71+
}
72+
},
5673
};
5774

5875
export default connect({

client/test/domain.test.js

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
// Copyright (c) 2017-2023 Uber Technologies Inc.
2+
//
3+
//
4+
// Permission is hereby granted, free of charge, to any person obtaining a copy
5+
// of this software and associated documentation files (the "Software"), to deal
6+
// in the Software without restriction, including without limitation the rights
7+
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
8+
// copies of the Software, and to permit persons to whom the Software is
9+
// furnished to do so, subject to the following conditions:
10+
//
11+
// The above copyright notice and this permission notice shall be included in
12+
// all copies or substantial portions of the Software.
13+
//
14+
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
15+
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
16+
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
17+
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
18+
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
19+
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
20+
// THE SOFTWARE.
21+
22+
import { getFixture } from './helpers';
23+
24+
const crossRegionSpecificFlags = [
25+
'crossRegion',
26+
'crossRegion.activeStatusTag',
27+
'crossRegion.allowedCrossOrigin',
28+
'crossRegion.clusterOriginList',
29+
];
30+
31+
const featureFlagsForMultipleClustersEnv = [
32+
...getFixture('featureFlags').filter(
33+
({ key }) => !crossRegionSpecificFlags.includes(key)
34+
),
35+
{
36+
key: 'crossRegion',
37+
value: true,
38+
},
39+
{
40+
key: 'crossRegion.activeStatusTag',
41+
value: true,
42+
},
43+
{
44+
key: 'crossRegion.allowedCrossOrigin',
45+
value: true,
46+
},
47+
{
48+
key: 'crossRegion.clusterOriginList',
49+
value: [
50+
{
51+
clusterName: 'primary',
52+
origin: 'http://localhost:8090',
53+
},
54+
{
55+
clusterName: 'ci-test-cluster',
56+
origin: 'http://localhost:8090',
57+
},
58+
],
59+
},
60+
];
61+
62+
const featureFlagsForSingleClusterEnv = [
63+
...getFixture('featureFlags').filter(
64+
({ key }) => !crossRegionSpecificFlags.includes(key)
65+
),
66+
{
67+
key: 'crossRegion',
68+
value: false,
69+
},
70+
{
71+
key: 'crossRegion.activeStatusTag',
72+
value: false,
73+
},
74+
{
75+
key: 'crossRegion.allowedCrossOrigin',
76+
value: false,
77+
},
78+
{
79+
key: 'crossRegion.clusterOriginList',
80+
value: [],
81+
},
82+
];
83+
84+
describe('Domain ', () => {
85+
async function domainTest(mochaTest, { desc, featureFlags } = {}) {
86+
const [testEl, scenario] = new Scenario(mochaTest)
87+
.withCluster()
88+
.withDomain('ci-test')
89+
.withDomainAuthorization('ci-test', true)
90+
.withFeatureFlags(featureFlags)
91+
.withEmptyNewsFeed()
92+
.withDomainDescription('ci-test', desc)
93+
.withWorkflows({ status: 'open' })
94+
.withWorkflows({ status: 'closed', startTimeOffset: 30 })
95+
.startingAt('/domains/ci-test')
96+
.go();
97+
98+
const configEl = await testEl.waitUntilExists('section.domain');
99+
100+
return [testEl, scenario];
101+
}
102+
103+
it('should redirect to cluster if it is missing in the url in a cross region domain environment', async function test() {
104+
// if clusterName is missing in the url and active cluser exists
105+
// make sure to redirect to add cluster to url
106+
// we make sure the activeCluster config exists by passing feature flags for crossRegion configs
107+
const [testEl, scenario] = await domainTest(this.test, {
108+
featureFlags: featureFlagsForMultipleClustersEnv,
109+
});
110+
111+
await testEl.waitUntilExists('.feature-flag .active-status');
112+
scenario.location.should.contain('/ci-test-cluster');
113+
});
114+
115+
it('should not redirect to cluster if it is missing in the url in a non cross region domain environment', async function test() {
116+
const [testEl, scenario] = await domainTest(this.test, {
117+
featureFlags: featureFlagsForSingleClusterEnv,
118+
});
119+
120+
// make sure that cross region is not in the dom before asserting on the location
121+
const statusEl = await testEl
122+
.waitUntilExists('.feature-flag .active-status')
123+
.catch(() => {
124+
scenario.location.should.not.contain('/ci-test-cluster');
125+
126+
return null;
127+
});
128+
129+
(statusEl === null).should.equal(true);
130+
});
131+
});

client/test/index.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ require('./scenario');
155155

156156
mocha.checkLeaks();
157157

158+
require('./domain.test');
158159
require('./domain-search.test');
159160
require('./help.test');
160161
require('./workflow-list.test');

client/test/scenario.js

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ Scenario.prototype.withDomainDescription = function withDomainDescription(
163163
) {
164164
const { origin } = this;
165165

166-
this.api.getOnce(
166+
this.api.get(
167167
`${origin}/api/domains/${domain}`,
168168
deepmerge(
169169
{
@@ -215,10 +215,15 @@ Scenario.prototype.withFeatureFlags = function withFeatureFlags(
215215
const { origin } = this;
216216

217217
featureFlags.forEach(({ key, value }) => {
218-
this.api.getOnce(`${origin}/api/feature-flags/${key}`, {
219-
key,
220-
value,
221-
});
218+
this.api.getOnce(
219+
`${origin}/api/feature-flags/${key}`,
220+
{
221+
key,
222+
value,
223+
},
224+
{ query: {} }
225+
); // for some reason when value contains an array of objects that has `origin` field it adds it to query params
226+
// so we need to reset query tp empty obj
222227
});
223228

224229
return this;
@@ -274,7 +279,7 @@ Scenario.prototype.withWorkflows = function withWorkflows({
274279
? { executions: workflows, nextPageToken: '' }
275280
: workflows;
276281

277-
this.api.getOnce(url, response);
282+
this.api.get(url, response);
278283

279284
return this;
280285
};

server/test/domain.test.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ describe('Describe Domain', function() {
4848
},
4949
replicationConfiguration: {
5050
activeClusterName: 'ci-cluster',
51-
clusters: [],
51+
clusters: [{ clusterName: 'ci-cluster' }],
5252
},
5353
},
5454
],
@@ -67,7 +67,7 @@ describe('Describe Domain', function() {
6767
visibilityArchivalStatus: 'ARCHIVAL_STATUS_INVALID',
6868
visibilityArchivalUri: '',
6969
activeClusterName: 'ci-cluster',
70-
clusters: [],
70+
clusters: [{ clusterName: 'ci-cluster' }],
7171
failoverVersion: '0',
7272
isGlobalDomain: false,
7373
failoverInfo: null,

0 commit comments

Comments
 (0)