Skip to content
This repository was archived by the owner on Feb 26, 2025. It is now read-only.

4641 // Pin history version to fix issue with opening ids with encoded characters #1478

Merged
merged 2 commits into from
Jan 22, 2024

Conversation

Dinika
Copy link
Contributor

@Dinika Dinika commented Jan 19, 2024

Fixes #4641

Description

This is meant to be a temporary solution until we update react-router to v6 (where the following issue is fixed with some caveats).

If a resource id has some encoded chars (like - https://hello.lol/https%3A%2F%2Fencoded.url%2Fwow),

history.push pushes this to window.location:

https%3A%2F%2Fhello.lol%2Fhttps%3A%2F%2Fencoded.url%2Fwow

instead of:

https%3A%2F%2Fhello.lol%2Fhttps%253A%252F%252Fencoded.url%252Fwow

Using an older version of history and forcing all fusion dependencies to use this version is working for now.
I tried pinning version 4.7 of history but that didn't work.

This solution is taken from remix-run/history#505 (comment)

Thanks to @bilalesi for sharing the solution

How to test this PR

  1. Run yarn to install the older version of history

  2. With fusion using dev version of nexus, try going to http://localhost:8000/copies/sscx/resources/https%3A%2F%2Fdev.nise.bbp.epfl.ch%2Fnexus%2Fv1%2Fresources%2Fcopies%2Fsscx%2F_%2F1bd3d9a5-7533-4033-b575-1823862c7b8c This should work

  3. If you search for the above resource in MyData or Data-Explorer or project listing views and click on its row, then it should open the resource container with the right resource instead of showing an error.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added necessary unit and integration tests.
  • I have added screenshots (if applicable), in the comment section.

@Dinika Dinika force-pushed the 4641/use-different-history-version branch from 4f651c6 to 54e56a5 Compare January 19, 2024 11:12
@codecov-commenter
Copy link

codecov-commenter commented Jan 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a15e5f1) 47.89% compared to head (df80ffb) 47.89%.
Report is 3 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #1478   +/-   ##
========================================
  Coverage    47.89%   47.89%           
========================================
  Files          250      250           
  Lines        11474    11474           
  Branches      2696     2696           
========================================
  Hits          5495     5495           
  Misses        5946     5946           
  Partials        33       33           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

     automatically decoded

This is a temporary solution until we update react-router to v6

If a nexus id has some encoded chars (like - https://hello.lol/https%3A%2F%2Fencoded.url%2Fwow),

history.push pushes this to window.location:
https%3A%2F%2Fhello.lol%2Fhttps%3A%2F%2Fencoded.url%2Fwow

instead of:
https%3A%2F%2Fhello.lol%2Fhttps%253A%252F%252Fencoded.url%252Fwow

Using an older version of `history` and forcing all fusion dependencies
to use this version is working for now.

This solution is taken from remix-run/history#505 (comment)

Signed-off-by: Dinika Saxena <dinikasaxenas@gmail.com>
@Dinika Dinika force-pushed the 4641/use-different-history-version branch from 1957584 to df80ffb Compare January 22, 2024 08:32
@bilalesi
Copy link
Contributor

nice @Dinika 👍

@Dinika Dinika merged commit dbf130d into develop Jan 22, 2024
1 check passed
@Dinika Dinika deleted the 4641/use-different-history-version branch January 22, 2024 10:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants