Conversation
| tabulateEntityA | ||
| :: (Applicative f) | ||
| => (forall a. EntityField record a -> f a) | ||
| => (forall a. PersistField a => EntityField record a -> f a) |
There was a problem hiding this comment.
This isn't an acceptable change imo
There was a problem hiding this comment.
we should have something like FieldDict in prairie - then use-sites would have FieldDict PersistField rec
There was a problem hiding this comment.
This isn't an acceptable change imo
Can you explain why? Even if there is a better way to support functions like assignAll and copyAll, does it hurt to add this constraint?
I'm not familiar with prairie, but it seems doing it properly would involve replacing PersistField with prairie's Field. That's a bigger change then what I'm comfortable taking on at the moment.
| copyAll = snd $ runWriter $ tabulateEntityA $ \field -> | ||
| -- This assumes the tabulateEntityA implementation is not strict in the | ||
| -- returned field value (and the default implementation indeed isn't). | ||
| error "copyAll: field value was used" | ||
| <$ when | ||
| ( fieldHaskell (persistFieldDef field) | ||
| /= fieldHaskell (persistFieldDef @record persistIdField) | ||
| ) | ||
| (tell [copyField field]) |
There was a problem hiding this comment.
this form feels very strange- i believe there should be a better way to write that 🤔 I know prairie has the right utilities but I'm not sure if persistent has them
There was a problem hiding this comment.
I don't know of any persistent utilities that could help here, but I'm open to more specific suggestions.
2 participants
This PR adds 2 functions
assignAllandcopyAllintended to be used when you need to insert a record into the database, and if it already exists, completely overwrite it. These can be implemented generically if we add aPersistField aconstraint intabulateEntityA. I think it's unlikely that this breaks existing code, so I propose a minor version increment.The implementation is slightly hacky because the
tabulateEntityAwants to build anEntity, but neitherassignAllnorcopyAllrequire full entity data. I think adding a method toPersistEntitysimilar tofieldLenswhich works overrecordinstead ofEntity recordis also worth considering, but for now I kept with the minimal change.I also fixed the nix flake to account for the "new"
persistent-postgresql => postgresql-simple-interval>=1dependency.Before submitting your PR, check that you've:
@sincedeclarations to the Haddockfourmoluon any changed files (restyledwill do this for you, soaccept the suggested changes if it makes them)
.editorconfigandfourmolu.yamlfiles for details)After submitting your PR:
(unreleased)on the Changelog