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

DOP-1549: Fix version dropdown links #287

Merged
merged 2 commits into from
Sep 23, 2020
Merged

DOP-1549: Fix version dropdown links #287

merged 2 commits into from
Sep 23, 2020

Conversation

sophstad
Copy link
Member

@sophstad sophstad commented Sep 22, 2020

[DOP-1549] [Charts Staging] [BI Connector Staging] Use values from the git.branches.published table of the published-branches.yaml file. These are the values that correspond to the git branches of each version, and also to the slug that should appear in the URL. Previously, the component was using the values that appear in the version.active table, which understandably worked out badly!

Copy link
Contributor

@schmalliso schmalliso left a comment

Choose a reason for hiding this comment

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

One question!

{active.map(version => {
const url = normalizePath(`${generatePrefix(version)}/${slug}`);
{Object.entries(gitNamedMapping).map(([branch, name]) => {
const url = normalizePath(`${generatePrefix(branch)}/${slug}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the branch and slug backwards?

The link is https://docs-mongodbcom-staging.corp.mongodb.com/19.12/charts/sophstad/master/ and I would have expected /charts/19.12/sophstad, but I also may be confused.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is indeed a matter of staging vs. production; for production builds, we use this logic:

if (pathPrefix) {
  const [project] = pathPrefix.split('/');
  return `/${project}/${version}`;
}

But this led me to notice another bug that could result from PATH_PREFIX not including a preceding slash, so I fixed that and added a test to ensure we are generating the correct link!

Copy link
Contributor

@schmalliso schmalliso left a comment

Choose a reason for hiding this comment

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

Oh of course the slug is there because we flip to the specific page. Thanks for the explain and yay for tests!

@sophstad sophstad merged commit f207084 into master Sep 23, 2020
@sophstad sophstad deleted the DOP-1549 branch September 23, 2020 12:57
graysonhicks pushed a commit that referenced this pull request Jan 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants