Skip to content
This repository has been archived by the owner on Aug 3, 2023. It is now read-only.

Add local likes & history to cloud after signing in #242

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add local likes & history to cloud after signing in #242

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e5add34
Add local like & history to cloud after signing in
aabounegm May 8, 2022
405d2aa
Fix authentication unit tests
aabounegm May 8, 2022
f3eeb8c
Add missing type annotation to fix inference
aabounegm May 10, 2022
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
5 changes: 5 additions & 0 deletions src/lib/features/authenticate/model/sign-in-snackbar.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { snackbar } from '$lib/shared/ui/snackbar';
import { supabaseClient } from '$lib/shared/api';
import { likesStore, addCloudLike } from '$lib/features/like-episode';
import { listeningHistory, addToCloudListeningHistory } from '$lib/features/listening-history';
Copy link
Collaborator

Choose a reason for hiding this comment

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

issue: FSD, cross-imports of features

Copy link
Owner Author

Choose a reason for hiding this comment

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

Same as #241 (comment)

import { get } from 'svelte/store';

export function signInSnackbar() {
const url = new URL(window.location.href);
Expand All @@ -14,5 +17,7 @@ export function signInSnackbar() {
});
url.searchParams.delete('snackbar');
window.history.replaceState(null, '', url.toString());
get(likesStore).forEach(addCloudLike);
Copy link
Collaborator

Choose a reason for hiding this comment

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

question: why did you decide to do it here, of all the places?)

suggestion: the same as in that other PR, let's do it here:

cast/src/lib/features/like-episode/model/like.ts

Lines 13 to 18 in bd8fc73

user.subscribe(async ($user) => {
if ($user) {
const ids = await fetchLikes();
likesStore.set(ids ?? []);
}
});

Copy link
Owner Author

Choose a reason for hiding this comment

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

Because I didn't want to add the like to Supabase every time the user loads the page (I'm not sure if there are any uniqueness constraints set in the backend). This seemed like the only function where I guarantee the user has just signed in after being logged out before and having added some episode to the favorites.
I agree it's not the best solution, especially that it only works if the user signed in through the snackbar that appears after liking an episode unauthenticated, but I also couldn't think of another quick solution that doesn't involve querying the backend likes/history and diffing the data.

Copy link
Collaborator

Choose a reason for hiding this comment

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

There are uniqueness constraints on the data, so uploading a duplicate like/history won't really change anything except for the created_at timestamp, which we don't use anyway, except for ordering.

Technically, we could have another derived store that tracks the previous value of the user store, and expose to the public yet another derived store that provides access to them both, however, that is slightly janky, so proceed at will

Copy link
Owner Author

Choose a reason for hiding this comment

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

Still, I would prefer not to depend on constraints in the backend, especially that the function name literally says "add". It's better to validate the data on the client side first and consider the backend validation as a safety measure

Copy link
Collaborator

Choose a reason for hiding this comment

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

Alright, then diffing on the client it is. Shouldn't be too expensive/hard

get(listeningHistory).forEach(addToCloudListeningHistory);
}
}
1 change: 1 addition & 0 deletions src/lib/features/authenticate/model/specs/sign-in.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { supabaseClient } from '$lib/shared/api';
import { signIn } from '../..';

jest.mock('$app/env', () => ({ browser: false }), { virtual: true });
jest.mock('$lib/shared/api', () => ({
...jest.requireActual('$lib/shared/api'),
supabaseClient: {
Expand Down
1 change: 1 addition & 0 deletions src/lib/features/authenticate/model/specs/sign-out.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { supabaseClient } from '$lib/shared/api';
import { signOut } from '../..';

jest.mock('$app/env', () => ({ browser: false }), { virtual: true });
jest.mock('$lib/shared/api', () => ({
...jest.requireActual('$lib/shared/api'),
supabaseClient: {
Expand Down
2 changes: 2 additions & 0 deletions src/lib/features/authenticate/model/specs/track-auth-status.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { user } from '$lib/entities/user';
import { supabaseClient } from '$lib/shared/api';
import { trackAuthStatus } from '../..';

jest.mock('$app/env', () => ({ browser: false }), { virtual: true });
jest.mock('$lib/shared/api', () => ({
...jest.requireActual('$lib/shared/api'),
supabaseClient: {
Expand All @@ -19,6 +20,7 @@ jest.mock('$lib/shared/api', () => ({
jest.mock('$lib/entities/user', () => ({
user: {
set: jest.fn().mockName('user.set() from $lib/entities/user'),
subscribe: jest.fn().mockName('user.subscribe() from $lib/entities/user'),
},
}));

Expand Down
1 change: 1 addition & 0 deletions src/lib/features/like-episode/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export { default as LikeButton } from './ui/like-button.svelte';
export { likesStore, toggleLike } from './model/like';
export { populateLikes } from './model/populate-likes';
export { addCloudLike } from './api/favourites-table';
1 change: 1 addition & 0 deletions src/lib/features/listening-history/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
export { addToListeningHistory, listeningHistory } from './model/store';
export { populateListeningHistory } from './model/populate-listening-history';
export { addToCloudListeningHistory } from './api/history-table';