Skip to content

Save custom decks to the database#818

Merged
marcelpanse merged 35 commits intomarcelpanse:mainfrom
greg19:save-decks
Jan 5, 2026
Merged

Save custom decks to the database#818
marcelpanse merged 35 commits intomarcelpanse:mainfrom
greg19:save-decks

Conversation

@greg19
Copy link
Contributor

@greg19 greg19 commented Dec 23, 2025

Fixes #735.

@greg19
Copy link
Contributor Author

greg19 commented Dec 23, 2025

@marcelpanse can you create the DB table? I think this will be okay:

CREATE TABLE decks (
  id SERIAL PRIMARY KEY,
  email VARCHAR(?) NOT NULL REFERENCES accounts(email) ON UPDATE CASCADE,
  is_public BOOLEAN NOT NULL,
  name VARCHAR(64) NOT NULL CHECK (length(name) > 0),
  energy VARCHAR(24)[] NOT NULL CHECK (array_length(energy, 1) > 0),
  cards INTEGER[] NOT NULL
);

Unrelated note: Can you verify that card_amounts table has email foreign key set? I can't see it

@marcelpanse
Copy link
Owner

Table is created.

Seems there are no FKs to the accounts table. Not sure why they were never created, best guess is because I started initially on Appwrite which didn't really have FKs and later migrated to Supabase.

@greg19
Copy link
Contributor Author

greg19 commented Dec 30, 2025

@marcelpanse can you also create a public_decks view?

@marcelpanse
Copy link
Owner

@marcelpanse can you also create a public_decks view?

Created public_decks for you.

create view public.public_decks(name, energy, cards) as
SELECT d.name,
       d.energy,
       d.cards
FROM decks d
WHERE d.is_public = true;

@greg19
Copy link
Contributor Author

greg19 commented Dec 30, 2025

If we want to support sharing a deck via a link /decks/:id then the id also has to be included

@marcelpanse
Copy link
Owner

If we want to support sharing a deck via a link /decks/:id then the id also has to be included

done!

@greg19
Copy link
Contributor Author

greg19 commented Jan 3, 2026

@marcelpanse can you also do:

CREATE TABLE deck_likes (
  id INTEGER NOT NULL REFERENCES decks(id),
  email VARCHAR(?) NOT NULL REFERENCES accounts(email) ON UPDATE CASCADE,
  PRIMARY KEY (deck_id, email)
);

This will serve two purposes. The first is bookmarking a deck, I want to also shown liked decks on "my decks" tab (or have 4th tab with the liked decks).

The second purpose is searching for decks. The "community decks" would have the decks sorted by the number of likes, so you won't be shown junk (that will inevitably appear if the users are allowed to post arbitrary decks). For this, we need to somehow count the likes. Counting them every time someone fetches the table isn't scalable, so we need to do some bookkeeping in the db.

CREATE TABLE deck_like_counts (
  id INTEGER PRIMARY KEY REFERENCES decks(id),
  likes INTEGER NOT NULL DEFAULT 0
);

CREATE INDEX deck_like_counts_order_idx
  ON deck_like_counts (likes DESC)
  INCLUDE (deck_id);

We can't give write access for users for this table. So liking and disliking a deck either has to be a RPC / deno call, or we need the triggers:

REVOKE ALL ON deck_like_counts FROM PUBLIC;
GRANT SELECT ON deck_like_counts TO PUBLIC;

CREATE OR REPLACE FUNCTION deck_likes_after_insert()
RETURNS TRIGGER
LANGUAGE plpgsql
SECURITY DEFINER
SET search_path = pg_catalog
AS $$
BEGIN
  INSERT INTO public.deck_like_counts (deck_id, likes)
  VALUES (NEW.deck_id, 1)
  ON CONFLICT (deck_id)
  DO UPDATE SET likes = deck_like_counts.likes + 1;

  RETURN NULL;
END;
$$;

CREATE OR REPLACE FUNCTION deck_likes_after_delete()
RETURNS TRIGGER
LANGUAGE plpgsql
SECURITY DEFINER
SET search_path = pg_catalog
AS $$
BEGIN
  UPDATE public.deck_like_counts
     SET likes = GREATEST(likes - 1, 0)
   WHERE deck_id = OLD.deck_id;

  RETURN NULL;
END;
$$;

CREATE TRIGGER trg_deck_likes_after_insert
AFTER INSERT ON deck_likes
FOR EACH ROW
EXECUTE FUNCTION deck_likes_after_insert();

CREATE TRIGGER trg_deck_likes_after_delete
AFTER DELETE ON deck_likes
FOR EACH ROW
EXECUTE FUNCTION deck_likes_after_delete();

@greg19
Copy link
Contributor Author

greg19 commented Jan 4, 2026

An alternative to a separate deck_like_counts table would be to have the like count column in the decks table with column level security. Supabase says CLS is an "advanced feature" and isn't recommended for "most users". I don't really understand their rationale, so I don't know if this is a "justified" use case (but treating this advice as "never use CLS" also seems wrong). IMO it would be much cleaner, as you always want to fetch the likes count with the deck, so we would do a join every time (which slightly hurts performance). I'll leave this decision up to you.

@marcelpanse
Copy link
Owner

I created the decks_likes table and I added the likes in the public_decks view using a join. So doing select * from public_decks will give you a column likes that you can sort on.

I think performance-wise this will be fine. I don't expect that many decks being made and liked, so that shouldn't be a problem at all.

If it does become super-popular somehow and the query becomes slow, then the best way to solve it is to make another cron job like the stats-tracker function that runs once per hour, does the query, and stores the results to a static json file. Then the app will load the static file to show the popular decks. This approach is super scalable and doesn't add any complexity to the database. The added benefit is that the like_count can never be out of sync with the actual deck_like table since it works on the actual data instead of some increments.

@greg19 greg19 marked this pull request as ready for review January 5, 2026 15:23
@greg19
Copy link
Contributor Author

greg19 commented Jan 5, 2026

I think I'm mostly done, it now can be tested and reviewed.

The Game8 scraper has been broken for a while now, and the scraped decks have different interface than the DB decks, which is a burden to maintain. I'm optimistic that in the long run the community decks will be enough to serve everyone. How do we stand about it? Personally, I don't want to touch the scraper, but I can adjust the decks page to show them if they are really needed (though even the current decks are outdated and new ones can't be fetched). Do we keep maintaining the scraper or can can it be dropped with this PR?

Search page

image image image

Deck details

image

Deck validation

image

greg19 added 4 commits January 5, 2026 16:49
Sets the default name of a new deck to an empty string to motivate the
user to set the name to something different than "New deck"
Comment on lines +58 to +72
useEffect(() => {
if (shouldFetch) {
setIsLoading(true)
setIsError(false)
getDeck(Number(deckId))
.then((deck) => {
setDeck(deck)
setIsLoading(false)
})
.catch(() => {
setIsLoading(false)
setIsError(true)
})
}
}, [shouldFetch, deckId])
Copy link
Owner

Choose a reason for hiding this comment

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

Why not use a useDeck() in react-query with a enabled: !!deckId
Then it should auto-fetch and refresh when stale and you can use the error/loading props directly from the useDeck() instead of manually managing this and using a useEffect.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because you need to be able to edit the deck, which itself requires a useState. Then useEffect is simpler than trying to put the useDeck here (useQuery doesn't produce any event when it fetches the data, so you have no place to call the initial setDeck after fetching form the db)

@marcelpanse
Copy link
Owner

Hey, looks good overal. I have some small remarks/improvements, but I can add that to a separate ticket so it doesn't block this release.

@marcelpanse
Copy link
Owner

Regarding the scraper, maybe we can adjust it to import the meta decks into the database (like in my account for example). We can also see how this will be used, maybe some people will start adding the meta decks themselves. So lets keep the scraper file for now and decide later.

@greg19
Copy link
Contributor Author

greg19 commented Jan 5, 2026

Okay, as you probably seen I've created some decks. Though deck id=8 is behaving weirdly, it no longer shows in my decks, and when I try to like it I get an error new row violates row-level security policy for table "deck_likes". Also, can you give me the read access in postgres to the newly created tables?

@marcelpanse
Copy link
Owner

I realized there was no RLS yet for the decks and decks-likes tables so I added them. The deck 8 I copied and edited before adding the RLS, so that's probably the issue.

when I now try to copy and edit your deck, it will fail because it seems it tries to edit the deck instead of making a new copy of it. Can you fix that?

I changed some of the RLS things, could you check if you still have readonly access to the other tables?

@greg19
Copy link
Contributor Author

greg19 commented Jan 5, 2026

I now don't have access to any of the tables

@greg19
Copy link
Contributor Author

greg19 commented Jan 5, 2026

when I now try to copy and edit your deck, it will fail because it seems it tries to edit the deck instead of making a new copy of it. Can you fix that?

Should be fixed

@marcelpanse
Copy link
Owner

I now don't have access to any of the tables

you should have access now to all tables

@marcelpanse
Copy link
Owner

After copying a deck and saving it, it doesn’t appear in my decks, only after a refresh (probably need to invalidate the query key)

@greg19
Copy link
Contributor Author

greg19 commented Jan 5, 2026

Fixed

@marcelpanse marcelpanse merged commit 2480b84 into marcelpanse:main Jan 5, 2026
1 check passed
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.

Deckbuilding mode filtering is inaccurate

2 participants