Add downgrade logic for branch in DocLinkService

[SuZhou-Joe]

Description

This is a TODO after PR.
The CoreStart.docLinks.docVersion is using pkg.branch. In 2.5 we changed it manually.
In this PR I want to add a mechanism into CI flow to prevent human error(If build team forget to change the value, which is 2.x now in current build flow, it will break down all the new doc links inside IM plugin because the link will become like https://opensearch.org/docs/2.x/api-reference/index-apis/create-index/.).

Issues Resolved

#3490 #3490

Check List

• All tests pass
  • yarn test:jest
  • yarn test:jest_integration
  • yarn test:ftr
• New functionality includes testing.
• New functionality has been documented.
• Update CHANGELOG.md
• Commits are signed per the DCO using --signoff

[SuZhou-Joe]

Please help to backport to 2.x and main.

[joshuarrrr]

@SuZhou-Joe Can you make an issue for this (see https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/CONTRIBUTING.md#contributing-code)? It's a great find and definitely a bug we should fix, but I think we may want to discuss other solutions. For example:

1. Updating the docs site routing so that 2.x URLs resolve to the latest released version
2. Updating the logic to resolve 2.x branches such as in https://github.com/joshuarrrr/OpenSearch-Dashboards/blob/main/src/core/public/doc_links/doc_links_service.ts#L42-L45

[joshuarrrr]

cc @AMoo-Miki who is keeping 👀 on all things version related.

[SuZhou-Joe]

@SuZhou-Joe Can you make an issue for this (see https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/CONTRIBUTING.md#contributing-code)? It's a great find and definitely a bug we should fix, but I think we may want to discuss other solutions. For example:

1. Updating the docs site routing so that 2.x URLs resolve to the latest released version
2. Updating the logic to resolve 2.x branches such as in https://github.com/joshuarrrr/OpenSearch-Dashboards/blob/main/src/core/public/doc_links/doc_links_service.ts#L42-L45

Thanks @joshuarrrr , the issue has been created and linked.
For option 1, we would like to use fixed version for specific version, for example when users are using the 2.4 version build, they should be redirect to opensearch.org/2.4/some_doc to prevent confusion when break changes happen on the API.
For option 2, yes it may work as well but I think we should put it in build layer instead of on server runtime but if you insist, I will change the implementation.

[codecov-commenter]

Codecov Report

Merging #3483 (f0d6198) into main (02e0bc1) will increase coverage by 0.00%.
The diff coverage is 100.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@           Coverage Diff           @@
##             main    #3483   +/-   ##
=======================================
  Coverage   66.45%   66.46%           
=======================================
  Files        3205     3208    +3     
  Lines       61562    61602   +40     
  Branches     9497     9503    +6     
=======================================
+ Hits        40912    40943   +31     
- Misses      18353    18383   +30     
+ Partials     2297     2276   -21     

Flag	Coverage Δ
Linux	66.40% <100.00%> (+<0.01%)	⬆️
Windows	66.41% <100.00%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/core/public/doc_links/doc_links_service.ts	100.00% <100.00%> (ø)

... and 26 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[SuZhou-Joe]

Please add backport 2.x label for this PR. This should be merged to 2.x as well.

[kavilla]

Hello @SuZhou-Joe,

Thanks for opening!

From my understanding it takes the config value and automatically sets it based on the config build version if not main to which point I kind of wonder what's the point of even making branch a config value.

I believe the branch is only used within the doc links service within our repo but I'm not positive if external plugins utilize this configuration for information.

If the purpose is for the doc links service, I'd recommend handling this logic https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/src/core/public/doc_links/doc_links_service.ts#L43.

Also, I can kind of see how we build the doc links service to be very unique to the branching strategy and doc link URL path set by the organization. I think it's fine not to worry about for now but if I was to fork this and change the URL I think it should be valid to pass suffix. I should even be able to pass my own site. But like I said, that's not something for you something that we should think about.

Let me know if you have any questions!

Thanks!

[kavilla]

Does this prevent the builds if they pass a non-valid doc paths pattern in the branch in the package?

[SuZhou-Joe]

Yes, if no version can be specified after so many downgrade logic, we will prevent the build because there will be a doc link error if we continue to build.

[SuZhou-Joe]

When the logic goes to here, the dashboard will display such error page. But it won't be here because dashboard will check the version with OpenSearch core and thus it will always be parsed successfully by semver.

[kavilla]

Thank you!

[abbyhu2000]

@SuZhou-Joe Can't we simplify line 54 to line 62 like this? since we already set let branch = pkgBranch; outside the block and line 57 and line 61 are doing the duplicate thing

      const validDocPathsPattern = /^\d+\.\d+$/;
      const parsedBuildVersion = parse(buildVersion);
      
      if (!validDocPathsPattern.test(pkgBranch) &&& parsedBuildVersion) {
        branch = `${parsedBuildVersion.major}.${parsedBuildVersion.minor}`;
      } 

[SuZhou-Joe]

agree and done.