psteinroe / supabase-cache-helpers

A collection of framework specific Cache utilities for working with Supabase.
https://supabase-cache-helpers.vercel.app
MIT License
502 stars 31 forks source link

Query params sent when values are undefined #441

Open e-lobo opened 6 months ago

e-lobo commented 6 months ago

Describe the bug In the hook useUpsertMutation when we pass in the payload with an object with values undefined the columns are sent to supabase.

Note: This was working before fine. Its recently that this broke.

To Reproduce Let me give you my use case.

Given the below schema

model NotificationLog {
  id         String                      @id @default(dbgenerated("gen_random_uuid()")) @db.Uuid
  title    String
  }

Notice how the id is autogenerated so it is not mandatory to be sent along in the payload.

So,

When we send

import {
  useUpsertMutation,
} from '@supabase-cache-helpers/postgrest-swr';
const upsert = useUpsertMutation(supabase.from('NotificationLog'), ['id'], 'id');
const result = await upsert.trigger([{id:undefined, title:'Title'}]);

This fails with error "null value in column \"id\" of relation \"NotificationLog\" violates not-null constraint"

Upon inspection of the network request i noticed that the column id is sent even though in the payload it is undefined.

/rest/v1/NotificationLog?columns=%22id%22,%22title%22&select=id,title

Im not sure why its failing suddenly out of the blue. Maybe something changed in supabase ? I'm not sure if its a bug here and am a little concerned of my usage of this mutation. Please advice

uncvrd commented 6 months ago

There may be a bigger problem with the library but just so you're aware, the supabase-js client will default to setting values to null when passing an array to upsert. Which can be fixed by setting defaultToNull to false

https://supabase.com/docs/reference/javascript/upsert

Make missing fields default to null. Otherwise, use the default value for the column. This only applies when inserting new rows, not when merging with existing rows under ignoreDuplicates: false. This also only applies when doing bulk upserts.

I dont think this library exposes that capability unfortunately. For example this is how you do it using the js client directly:

            await ctx.supabase.from("CollectionService").upsert(
                services.map(({ id, ...service }, order) => {
                    const form = formatDbFields(service, nullableServiceFields);

                    const serviceId = Math.max(0, id);

                    return {
                        ...(!!serviceId && { id: serviceId }),
                        name: service.name === null ? ServiceName.CUSTOM : service.name,
                        customName: service.name === ServiceName.CUSTOM && !!service.customName ? service.customName : null,
                        collectionId,
                    };
                }),
                { defaultToNull: false }
            );

I dealt with this last week using the js client directly so just wanted to share since it was fresh on my mind and might help with updating the library here :)

uncvrd commented 6 months ago

oh wait yes you can, that property is exposed in the fourth parameter

    const upsert = useUpsertMutation(supabase.from("Team"), ["id"], "id", {
        defaultToNull: false
    })

Does something like that work? Also where is trigger coming from? You should probably be using mutate or mutateAsync?

e-lobo commented 6 months ago

Does something like that work? Also where is trigger coming from? You should probably be using mutate or mutateAsync?

i copy pasted some custom code so that how the trigger came in my bad.

defaultToNull: false

this works nice šŸ‘šŸ‘

so something did change in supabase for this to be mandatory now šŸ¤”

uncvrd commented 6 months ago

great! glad that worked. Not quite sure, I started working with Supabase JS in January of this year and its been around since then šŸ¤·