Skip to content

Fix/check childRoutes in getRoute#903

Merged
AinaVendrell merged 2 commits intomasterfrom
fix/getRoute-check-childRoutes
Nov 6, 2020
Merged

Fix/check childRoutes in getRoute#903
AinaVendrell merged 2 commits intomasterfrom
fix/getRoute-check-childRoutes

Conversation

@AinaVendrell
Copy link
Copy Markdown
Contributor

Remember to tag the PR with WIP tag if it's not ready to be merged.
Reference

Description

The function getRoute now checks if the input matches with a childRoute

Context

Some tests of botonic/core were failing because the function getRoute wasn't checking the childRoutes

Approach taken / Explain the design

Call getRoute recursively to check if the input matches with a childRoute

To documentate / Usage example

Testing

The pull request...

  • has unit tests
  • has integration tests
  • doesn't need tests because... [provide a description]

@AinaVendrell AinaVendrell changed the title Fix/get route check child routes Fix/check childRoutes in getRoute Aug 14, 2020
Copy link
Copy Markdown
Contributor

@ericmarcos ericmarcos left a comment

Choose a reason for hiding this comment

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

Sorry, I think I didn't explain well the desired behaviour.
When the router gets a "path payolad", that is, a payload in the format __PATH_PAYLOAD__<path> (for example __PATH_PAYLOAD__initial/step-2) this means that we should ignore lastRoutePath value and simply return the route identified by .

The 3 tests that were skipped shouldn't be changed, they reflect the intended behaviour (notice how you changed the 3rd test to match this implementation).

I think the solution could be very simple: just at the beginning of the processInput function, check if we're dealing with a "path payload" (__PATH_PAYLOAD__<path>) and just go find that ignoring the lastRoutePath. We already have a function getRouteByPath that I think you could reuse. Else, just leave the rest of logic as is.

@AinaVendrell AinaVendrell force-pushed the fix/getRoute-check-childRoutes branch from bdc2d0f to 9eb18ff Compare October 6, 2020 12:20
Copy link
Copy Markdown
Contributor

@ericmarcos ericmarcos left a comment

Choose a reason for hiding this comment

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

Nice!

@AinaVendrell AinaVendrell merged commit f237cf6 into master Nov 6, 2020
@AinaVendrell AinaVendrell deleted the fix/getRoute-check-childRoutes branch November 6, 2020 09:23
@AinaVendrell AinaVendrell mentioned this pull request Nov 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants