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

[BUGFIX] argilla frontend: redirect after login #5635

Merged
merged 45 commits into from
Nov 6, 2024
Merged
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
2a98e73
feat: change dataset publish validation from at least one required fi…
jfcalvo Oct 7, 2024
1427abb
chore: Update CHANGELOG.md
jfcalvo Oct 7, 2024
c25c88c
[Refactor] Remove settings name pattern restrictions (#5573)
frascuchon Oct 7, 2024
6cc4d07
Merge branch 'develop' into feat/argilla-direct-feature-branch
frascuchon Oct 7, 2024
16cbc35
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 7, 2024
ce59adc
Merge branch 'develop' into feat/argilla-direct-feature-branch
frascuchon Oct 14, 2024
6240d4f
chore: Fix tests after merge
frascuchon Oct 14, 2024
2ecca9d
feat: Add `metadata` support for datasets (#5586)
jfcalvo Oct 15, 2024
c9de8c3
Merge branch 'develop' into feat/argilla-direct-feature-branch
frascuchon Oct 16, 2024
ac29661
Merge branch 'develop' into feat/argilla-direct-feature-branch
jfcalvo Oct 17, 2024
8e85f50
feat: add server dataset hub import (#5591)
jfcalvo Oct 18, 2024
bc720c2
Update .github/workflows/argilla.yml
frascuchon Oct 18, 2024
d375c4b
feat: fix importing datasets features mapped as chat fields (#5611)
jfcalvo Oct 18, 2024
098d36b
feat: add support to use datasets using features with sequences of cl…
jfcalvo Oct 21, 2024
c916930
feat: add dataset split to be used along with row idx when `external_…
jfcalvo Oct 22, 2024
9b8e0ad
[REFACTOR] argilla: using split and row idx for default external (#5617)
frascuchon Oct 22, 2024
1eae163
Merge branch 'develop' into feat/argilla-direct-feature-branch
frascuchon Oct 22, 2024
eee62bb
fix: add missing image type check
jfcalvo Oct 23, 2024
a6535eb
Merge branch 'develop' into feat/argilla-direct-feature-branch
frascuchon Oct 23, 2024
7b85b45
Merge branch 'develop' into feat/argilla-direct-feature-branch
frascuchon Oct 23, 2024
3fbbc6e
[BUGFIX] argilla: support multi label values (#5625)
frascuchon Oct 25, 2024
c4afae1
Merge branch 'develop' into feat/argilla-direct-feature-branch
frascuchon Oct 28, 2024
1e8e160
add redirect router on HF welcome page
frascuchon Oct 28, 2024
3585a8f
go to redirect URL after OAuth sign-in
frascuchon Oct 28, 2024
f685afb
test: Check extra query paramters are returned on HF oauth callback
frascuchon Oct 28, 2024
0c31b24
uncomment code
frascuchon Oct 28, 2024
a0e4017
chore: Redirect after login
frascuchon Oct 28, 2024
b8ddd03
revert: router di to OAuthLoginUseCase
frascuchon Oct 28, 2024
55f3906
Update argilla-frontend/v1/domain/usecases/oauth-login-usecase.test.ts
frascuchon Oct 28, 2024
0b2194e
chore: remove comments
frascuchon Oct 28, 2024
444ccd9
Merge branch 'fix/argilla-frontend/redirect-after-login' of github.co…
frascuchon Oct 28, 2024
12ddf05
Merge branch 'develop' into fix/argilla-frontend/redirect-after-login
frascuchon Nov 5, 2024
af0d1cb
Update .github/workflows/argilla.yml
frascuchon Nov 5, 2024
fc8dfd4
Update argilla/CHANGELOG.md
frascuchon Nov 5, 2024
beba188
Update argilla-server/pdm.lock
frascuchon Nov 5, 2024
f09103a
Update argilla-server/pdm.lock
frascuchon Nov 5, 2024
f1e5b96
refactor: Using localStorage to keep the redirect url
frascuchon Nov 5, 2024
908f044
chore: Rename redirect to redirectTo
frascuchon Nov 5, 2024
fa2f89b
chore: Skip api calls on route guard
frascuchon Nov 6, 2024
6a5fcd5
fix: Pop instead of get redirectTo value
frascuchon Nov 6, 2024
41efb92
using redirect local store for user/password sign in
frascuchon Nov 6, 2024
1909e8f
chore: Update changelog
frascuchon Nov 6, 2024
275d161
chore: Remove unnecessary check
frascuchon Nov 6, 2024
d3c9e27
debug: Show route in console log
frascuchon Nov 6, 2024
ee445f2
refactor: Skip unknow routes
frascuchon Nov 6, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/argilla.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ jobs:
build:
services:
argilla-server:
image: argilladev/argilla-hf-spaces:develop
image: argilladev/argilla-hf-spaces:pr-5572
frascuchon marked this conversation as resolved.
Show resolved Hide resolved
ports:
- 6900:6900
env:
Expand Down
5 changes: 1 addition & 4 deletions argilla-frontend/middleware/route-guard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,9 @@ export default ({ $auth, route, redirect }: Context) => {
if (route.params.omitCTA) return;

if (isRunningOnHuggingFace()) {
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const { redirect: _, ...query } = route.query;

return redirect({
name: "welcome-hf-sign-in",
query,
query: route.query,
});
}
break;
Expand Down
11 changes: 10 additions & 1 deletion argilla-frontend/pages/oauth/_provider/useOAuthViewModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,28 @@ export const useOAuthViewModel = () => {
await tryLogin();
});

const redirect = () => {
let { redirect } = router.getQuery();
frascuchon marked this conversation as resolved.
Show resolved Hide resolved
if (Array.isArray(redirect)) {
redirect = redirect[0];
}

router.go(redirect || "/");
};

const tryLogin = async () => {
const { params, query } = routes.value;

const provider = params.provider as ProviderType;

try {
await oauthLoginUseCase.login(provider, query);
redirect();
} catch {
notification.notify({
message: t("argilla.api.errors::UnauthorizedError"),
type: "danger",
});
} finally {
router.go("/");
}
};
Expand Down
56 changes: 56 additions & 0 deletions argilla-server/pdm.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion argilla-server/tests/unit/api/handlers/v1/test_oauth2.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,14 +89,15 @@ async def test_provider_huggingface_authentication(
):
with mock.patch("argilla_server.security.settings.Settings.oauth", new_callable=lambda: default_oauth_settings):
response = await async_client.get(
"/api/v1/oauth2/providers/huggingface/authentication", headers=owner_auth_header
"/api/v1/oauth2/providers/huggingface/authentication?extra=params", headers=owner_auth_header
)
assert response.status_code == 303

redirect_url = URL(response.headers.get("location"))
assert redirect_url.scheme == b"https"
assert redirect_url.host == b"huggingface.co"
assert b"/oauth/authorize?response_type=code&client_id=client_id" in redirect_url.target
assert b"&extra=params" in redirect_url.target

async def test_provider_authentication_with_oauth_disabled(
self,
Expand Down
4 changes: 4 additions & 0 deletions argilla/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ These are the section headers that we use:

- Removed name sanitizing for dataset settings names. This may cause issues with old server versions. Especially when working with `from_hub`. ([#5574](https://github.com/argilla-io/argilla/pull/5574))

### Removed

- Removed name sanitizing for dataset settings names. This may cause issues with old server versions. Especially when working with `from_hub`. ([#5574](https://github.com/argilla-io/argilla/pull/5574))

frascuchon marked this conversation as resolved.
Show resolved Hide resolved
## [2.3.0](https://github.com/argilla-io/argilla/compare/v2.2.2...v2.3.0)

### Added
Expand Down
Loading