Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

RC > Config params as an object; always decode URL params #421

Merged
merged 5 commits into from
Jul 3, 2018
Merged
Show file tree
Hide file tree
Changes from 3 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
10 changes: 5 additions & 5 deletions runtime/auth-http.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ describe('SkyAuthHttp', () => {
provide: SkyAppConfig,
useValue: {
runtime: {
params: new SkyAppRuntimeConfigParams(url, [
'envid',
'leid',
'svcid'
])
params: new SkyAppRuntimeConfigParams(url, {
'envid': true,
'leid': true,
'svcid': true
})
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions runtime/bootstrapper.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe('bootstrapper', () => {

SkyAppBootstrapper.config = {
auth: true,
params: []
params: {}
};

SkyAppBootstrapper.processBootstrapConfig().then(() => {
Expand Down Expand Up @@ -65,7 +65,7 @@ describe('bootstrapper', () => {

it('should immediately resolve if SkyAppConfig.config.skyux.auth is not set', (done) => {
SkyAppBootstrapper.config = {
params: []
params: {}
};

SkyAppBootstrapper.processBootstrapConfig().then(done);
Expand All @@ -77,7 +77,7 @@ describe('bootstrapper', () => {

SkyAppBootstrapper.config = {
auth: true,
params: []
params: {}
};

SkyAppBootstrapper.processBootstrapConfig().then(() => {
Expand Down
7 changes: 6 additions & 1 deletion runtime/config-params.ts
Original file line number Diff line number Diff line change
@@ -1 +1,6 @@
export type SkyuxConfigParams = string[] | { [key: string]: boolean | { value?: any; required?: boolean } };

Choose a reason for hiding this comment

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

I'm not necessarily opposed, just don't recall talking explicitly about it. Was the intention here to remove support for an array of strings?

Copy link
Member Author

Choose a reason for hiding this comment

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

@Blackbaud-BobbyEarl I may've misunderstood the issue, but I took it to mean that SkyConfigParams would always be considered an object. This would make it easier to document and support, instead of having two different types.

https://github.com/blackbaud/skyux2/projects/9#card-10723415

Choose a reason for hiding this comment

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

No, you're right. That's a good catch regarding deprecating that syntax. Sorry for the confusion.

export type SkyuxConfigParams = {
[key: string]: boolean | {
value?: any;
required?: boolean
}
};
21 changes: 7 additions & 14 deletions runtime/params.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ import { SkyAppRuntimeConfigParams } from './params';

describe('SkyAppRuntimeConfigParams', () => {

const allowed = [
'a1',
'a3'
];
const allowed = {
'a1': true,
'a3': true
};

it('should parse allowed params from a url', () => {
const params: SkyAppRuntimeConfigParams = new SkyAppRuntimeConfigParams(
Expand Down Expand Up @@ -153,7 +153,7 @@ describe('SkyAppRuntimeConfigParams', () => {
});
});

it('should allow values to be decoded when retrieved from the query string', () => {
it('should decode values when retrieved from the query string', () => {
const params = new SkyAppRuntimeConfigParams(
'?a=%2F',
{
Expand All @@ -164,14 +164,8 @@ describe('SkyAppRuntimeConfigParams', () => {
}
);

// Preserves previous behavior of not encoding values from the query string.
expect(params.get('a')).toBe('%2F');
expect(params.get('b')).toBe('%2F');

// The second parameter tells the get() method to decode the parameter if it's from the
// query string.
expect(params.get('a', true)).toBe('/');
expect(params.get('b', true)).toBe('%2F');
expect(params.get('a')).toBe('/');
expect(params.get('b')).toBe('/');
});

it('should allow queryparam values to be required', () => {
Expand Down Expand Up @@ -256,5 +250,4 @@ describe('SkyAppRuntimeConfigParams', () => {

expect(params.getAllKeys()).toEqual([]);
});

});
71 changes: 25 additions & 46 deletions runtime/params.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,40 +29,28 @@ export class SkyAppRuntimeConfigParams {
url: string,
configParams: SkyuxConfigParams
) {
let allowed: string[];

// The default params value in Builder's skyuxconfig.json has been changed
// from an array to an object to support more metadata about each parameter,
// including the parameter's default value and possible future properties
// like required. Check for an array first to maintain backwards compatibility
// with the previous default value and any consumers who may be overriding the
// value until we release builder 2.0.
if (Array.isArray(configParams)) {
allowed = configParams;
} else {
allowed = [];

for (const paramName of Object.keys(configParams)) {
const configParam = configParams[paramName];

// The config param could be present but be set to false/undefined indicating
// an override of the default parameter.
if (configParam) {
allowed.push(paramName);

// A boolean value may be present to simply indicate that a parameter is allowed.
// If the type is object, look for additional config properties.
if (typeof configParam === 'object') {
const paramValue = configParam.value;

if (configParam.required) {
this.requiredParams.push(paramName);
}

if (paramValue) {
this.params[paramName] = paramValue;
this.defaultParamValues[paramName] = paramValue;
}
let allowed: string[] = [];

for (const paramName of Object.keys(configParams)) {
const configParam = configParams[paramName];

// The config param could be present but be set to false/undefined indicating
// an override of the default parameter.
if (configParam) {
allowed.push(paramName);

// A boolean value may be present to simply indicate that a parameter is allowed.
// If the type is object, look for additional config properties.
if (typeof configParam === 'object') {
const paramValue = configParam.value;

if (configParam.required) {
this.requiredParams.push(paramName);
}

if (paramValue) {
this.params[paramName] = paramValue;
this.defaultParamValues[paramName] = paramValue;
}
}
}
Expand Down Expand Up @@ -122,20 +110,11 @@ export class SkyAppRuntimeConfigParams {
* Returns the value of the requested param.
* @name get
* @param {string} key The parameter's key.
* @param {boolean} urlDecode A flag indicating whether the value should be URL-decoded.
* Specify true when you anticipate the value of the parameter coming from the page's URL.
* @returns {string}
*/
public get(key: string, urlDecode?: boolean): string {
public get(key: string): string {
if (this.has(key)) {
let val = this.params[key];

// This should be changed to always decode encoded params in skyux-builder 2.0.
if (urlDecode && this.encodedParams.indexOf(key) >= 0) {
val = decodeURIComponent(val);
}

return val;
return decodeURIComponent(this.params[key]);
}
}

Expand Down Expand Up @@ -179,7 +158,7 @@ export class SkyAppRuntimeConfigParams {

this.getAllKeys().forEach(key => {
if (!urlSearchParams.has(key)) {
joined.push(`${key}=${encodeURIComponent(this.get(key, true))}`);
joined.push(`${key}=${encodeURIComponent(this.get(key))}`);
}
});

Expand Down