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

fix: correctly handle settings/import link generation #47

Merged
merged 1 commit into from
Nov 26, 2021

Conversation

Ldoppea
Copy link
Member

@Ldoppea Ldoppea commented Nov 22, 2021

Like for the extension, the mobile app was not handling subdomain type correctly. Also it was still using the old route parameters (/installation/import vs /vault?action=import)

This PR fixes those problems

using System.Collections.Generic;
using System.Linq;
using System.Text;
using Flurl;
Copy link
Contributor

Choose a reason for hiding this comment

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

Flurl is an http client, right? Is it needed here?

Copy link
Member Author

@Ldoppea Ldoppea Nov 22, 2021

Choose a reason for hiding this comment

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

Nope, this one contains only the Url builder. Http client is served with Flurl.Http.

This is what I understand from their website : https://flurl.dev/
image

The native .net UrlBuilder seems not to provide any easy interface to manipulate QueryParams. I had the choice between constructing them manually or to use any known library like Flurl.
In fact for this specific import link it was not required, but I wanted to implement the same behavior than CozyClient.GenerateWebLink to be future proof.

Also I expect it to be useful when we will improve login url handling : https://trello.com/c/hJ6sa3Tn

We can still revert this choice.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't want us to manipulate QueryParams / String by hands. Let's use a lib for that :)

Settings' `Import` button now correctly redirects the user by handling
subdomain type (`nested` or `flat`) and by setting correct hash
@delete-merged-branch delete-merged-branch bot deleted the branch master November 26, 2021 12:11
@Ldoppea Ldoppea changed the base branch from merge-upstream_20211025 to master November 26, 2021 12:17
@Ldoppea Ldoppea merged commit 72d7754 into master Nov 26, 2021
@delete-merged-branch delete-merged-branch bot deleted the fix/import_vault branch November 26, 2021 12:18
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.

None yet

2 participants