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

Order channels by track/risk/branch #1581

Merged
merged 2 commits into from
Feb 11, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
262 changes: 262 additions & 0 deletions static/js/libs/channels.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,262 @@
const RISKS = ["stable", "candidate", "beta", "edge"];

/**
* Parse a channel string into an object
* @param {string} channelString
* @param {{defaultTrack: string}} options
* @returns {{format: {risk: boolean, track: boolean, branch: boolean}, risk: *, track: (string|*), branch: (string|*)}}
*/
function parseChannel(channelString, options) {
const format = {
track: false,
risk: false,
branch: false
};
const channelArr = ["latest", undefined, "_base"];
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason for doing this in array instead of 3 variables (track, risk, branch)?

Also isn't the default branch empty? If we start using "_base" we may end up detecting a branch when there isn't any. But maybe as we approach adding support for branches in more elements of UI we should just start using "_base" as default. If that's the case it would be good to have "_base" (but "latest" as well) defined somewhere as consts and reused instead of being copied.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I introduced the "_base" as a concept to have both a common format for all track/risks, regardless of whether they have branches and also to help with parsing in other parts in this file, for example reducing/ sorting ["stable/test", "stable"] was producing ["stable/test"] so stripping the "base" stable channel.

Setting "_base" hopefully makes it clear for anyone calling this code in the future (for another part of the site)


if (options) {
if (options.defaultTrack) {
channelArr[0] = options.defaultTrack;
}
}

const parts = channelString.split("/");
if (parts.length === 1) {
channelArr[1] = channelString;
format.risk = true;
} else if (parts.length === 2) {
if (RISKS.indexOf(parts[0]) === -1) {
channelArr[0] = parts[0];
channelArr[1] = parts[1];
format.track = true;
format.risk = true;
} else {
channelArr[1] = parts[0];
channelArr[2] = parts[1];
format.risk = true;
format.branch = true;
}
} else if (parts.length === 3) {
channelArr[0] = parts[0];
channelArr[1] = parts[1];
channelArr[2] = parts[2];
format.track = true;
format.risk = true;
format.branch = true;
}

return {
track: channelArr[0],
risk: channelArr[1],
branch: channelArr[2],
format: format
};
}

/**
* Create a tree from a list of channels
* @param {[{track: string, risk: string, branch: string, format: {track: boolean, branch: boolean, format: boolean}}]} channelList
* @returns {{
* track: {
* name: string,
* risks: {
* risk: {
* name: string,
* branches: {
* branch: {
* name: string
* }
* }
* }
* }
* }
* }}
*/
function createChannelTree(channelList) {
const tracks = {};

channelList.forEach(channel => {
if (!tracks[channel.track]) {
tracks[channel.track] = {
name: channel.track,
risks: {}
};
}

let level = tracks[channel.track];

if (!level.risks[channel.risk]) {
level.risks[channel.risk] = {
name: channel.risk
};
}

level = level.risks[channel.risk];

if (!level.branches) {
level.branches = {};
}

level.branches[channel.branch] = {
name: channel.branch
};
});

return tracks;
}

/**
* Sort a list of strings
* The output order will be:
* - hoistValue
* - strings
* - ascending
* - numbers
* - descending
* @param {[string]} list
* @param {string} hoistValue A value to always appear at the top
* @returns {*[]}
*/
function sortAlphaNum(list, hoistValue) {
let numbers = [];
let strings = [];
let latest = [];
list.forEach(item => {
// numbers are defined by any string starting any of the following patterns:
// just a number – 1,2,3,4,
// numbers on the left in a pattern – 2018.3 , 1.1, 1.1.23 ...
// or numbers on the left with strings at the end – 1.1-hotfix
if (hoistValue && item === hoistValue) {
latest.push(item);
} else if (isNaN(parseInt(item.substr(0, 1)))) {
strings.push(item);
} else {
numbers.push(item);
}
});

// Ignore case
strings.sort(function(a, b) {
return a[0].toLowerCase().localeCompare(b[0].toLowerCase());
});

strings = latest.concat(strings);

// Sort numbers (that are actually strings)
numbers.sort((a, b) => {
return b[0].localeCompare(a[0], undefined, {
numeric: true,
sensitivity: "base"
});
});

// Join the arrays together again

return strings.concat(numbers);
}

/**
* Sort channels into the following format:
* defaultTrack/stable
* defaultTrack/stable/branches
* defaultTrack/candidate
* defaultTrack/candidate/branches
* defaultTrack/beta
* defaultTrack/beta/branches
* defaultTrack/edge
* defaultTrack/edge/branches
* track/stable
* track/stable/branches
* ...
*
* @param {[string]} channels An array of "risk" / "track/risk" / "track/risk/branch"
* @param {{defaultTrack: string}} options
Copy link
Contributor

Choose a reason for hiding this comment

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

If there is no other option then defaultTrack maybe it just could be a defaultTrack param?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to make it generic for future and be more descriptive in the code that calls it. I find:
sortChannels([], "randomString") harder to parse than sortChannels([], { defaultTrack: "randomString" }) when scanning code... I'm open to arguments against though!

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if "randomString" would be used in such way directly in code. I would guess in most cases default track would be passed as a variable, so it would be something more like sortChannels([...], defaultTrack) which would be more clear.

But, I agree that unnamed option params may be confusing (especially when passing booleans), so I'm not against for sure.

In this case I just wondered if there is any reason to pass it via object. And because it seems like it's used like that consistently in at least 2 functions I'm fine with leaving it as it is.
Code is clear to read, and works, so not worth spending time changing it for little benefit.

* @returns {{tree: Array, list: Array}}
*/
function sortChannels(channels, options) {
const channelList = channels.map(channel => parseChannel(channel, options));
const channelTree = createChannelTree(channelList);

const sortedByTrack = [];

let track = "latest";
if (options && options.defaultTrack) {
track = options.defaultTrack;
}

const trackOrder = sortAlphaNum(Object.keys(channelTree), track);

trackOrder.forEach(track => {
sortedByTrack.push(channelTree[track]);
});

sortedByTrack.map(track => {
const riskOrder = Object.keys(track.risks).sort((a, b) => {
return RISKS.indexOf(a) - RISKS.indexOf(b);
});

track.risks = riskOrder.map(risk => track.risks[risk]);

track.risks.map(risk => {
const branchOrder = sortAlphaNum(Object.keys(risk.branches), "_base");

risk.branches = branchOrder.map(branch => risk.branches[branch]);

return risk;
});

return track;
});

const toArray = () => {
const list = [];
sortedByTrack.forEach(track => {
if (track.risks) {
track.risks.forEach(risk => {
if (risk.branches.length > 1 || risk.branches[0].name !== "_base") {
risk.branches.forEach(branch => {
const format = channelList.filter(
item =>
item.track === track.name &&
item.risk === risk.name &&
item.branch === branch.name
)[0].format;
const str = [];
if (format.track) {
str.push(track.name);
}
if (format.risk) {
str.push(risk.name);
}
if (format.branch) {
str.push(branch.name);
}
list.push(str.join("/"));
});
} else {
const format = channelList.filter(item => {
return item.track === track.name && item.risk === risk.name;
})[0].format;
const str = [];
if (format.track) {
str.push(track.name);
}
if (format.risk) {
str.push(risk.name);
}
list.push(str.join("/"));
}
});
}
});

return list;
};

return {
tree: sortedByTrack,
list: toArray()
};
}

export { sortChannels, parseChannel, createChannelTree, sortAlphaNum };