flyteconsole icon indicating copy to clipboard operation
flyteconsole copied to clipboard

fix: Broken breadcrumbs when using custom BASE_URL

Open ddl-rliu opened this issue 10 months ago • 3 comments

TL;DR

Typo – causes breadcrumbs to not appear in flyteconsole. To fix, use BASE_URL not BASE_PATH.

Type

  • [x] Bug Fix
  • [ ] Feature
  • [ ] Plugin

Are all requirements met?

  • [ ] Code completed
  • [ ] Smoke tested
  • [ ] Unit tests added
  • [ ] Code documentation added
  • [ ] Any pending items have an associated Issue

Complete description

Missing breadcrumbs for custom BASE_URL like /custom-base/console

image

Expectation: breadcrumbs are present for custom BASE_URL Flyte instance.

image

This happens because when BASE_PATH is used instead of BASE_URL, the logic that generates the breadcrumbs at breadcrumbRegistry.breadcrumbBuilder ends up using the wrong BASE_URL value (this starts from makeUrlSegments). This causes it to parse the window location incorrectly, and the resulting generated breadcrumbs is missing the expected entries.

Tracking Issue

Remove the 'fixes' keyword if there will be multiple PRs to fix the linked issue

fixes https://github.com/flyteorg/flyte/issues/

Follow-up issue

NA OR https://github.com/flyteorg/flyte/issues/

ddl-rliu avatar Apr 04 '24 19:04 ddl-rliu

Thank you for opening this pull request! 🙌

These tips will help get your PR across the finish line:

  • Most of the repos have a PR template; if not, fill it out to the best of your knowledge.
  • Sign off your commits (Reference: DCO Guide).

welcome[bot] avatar Apr 04 '24 19:04 welcome[bot]

@jsonporter , can you take a look?

eapolinario avatar Apr 06 '24 00:04 eapolinario

Codecov Report

Attention: Patch coverage is 0% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 54.76%. Comparing base (34edde1) to head (1db296c).

Files Patch % Lines
...nsole/src/components/Breadcrumbs/registry/index.ts 0.00% 1 Missing :warning:
Additional details and impacted files
@@           Coverage Diff           @@
##           master     #861   +/-   ##
=======================================
  Coverage   54.76%   54.76%           
=======================================
  Files         578      578           
  Lines       15137    15137           
  Branches     2760     2760           
=======================================
  Hits         8290     8290           
  Misses       6847     6847           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Apr 06 '24 00:04 codecov[bot]

Anything we can do to help get this typo fix merged @eapolinario / @jsonporter ?

ddl-ebrown avatar May 12 '24 21:05 ddl-ebrown

Congrats on merging your first pull request! 🎉

welcome[bot] avatar Jun 13 '24 22:06 welcome[bot]

:tada: This PR is included in version 1.15.1 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket:

flyte-bot avatar Jun 13 '24 23:06 flyte-bot