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

Removing post url doesnt federate (ref #4372) #4378

Closed
wants to merge 5 commits into from

Conversation

Nutomic
Copy link
Member

@Nutomic Nutomic commented Jan 15, 2024

Post url should be removed by setting it to Some(None) on PostUpdateForm. But for federation we use PostInsertForm as there is no way to distinguish between insert and update. The insert form doesnt allow setting Some(None), only Some(url) or None. So there is currently no way to set the url to null. Not sure whats the best way to fix this. The same problem might affect other fields as well.

@dullbananas
Copy link
Collaborator

This can be fixed by updating using a tuple with post::column_name.eq(excluded(post::column_name)) for each column.

https://docs.rs/diesel/latest/diesel/upsert/fn.excluded.html

https://www.postgresql.org/docs/current/sql-insert.html#SQL-ON-CONFLICT

This also makes it possible to later implement batched upserts because all rows use the same expressions for the update.

@Nutomic
Copy link
Member Author

Nutomic commented Jan 22, 2024

@dullbananas That doesnt sound correct, after all its valid to change the post url to a different one. But maybe you can implement it and see if it passes the tests.

@dullbananas
Copy link
Collaborator

@Nutomic I don't see why it would prevent changing the url. excluded gets the new row, not the old row.

@Nutomic
Copy link
Member Author

Nutomic commented Jan 22, 2024

Okay I understood it now and implemented that. There are two problems:

  • Its easy to forget adjusting the Post::update function when new fields are added, so maybe it should be done with a macro
  • As the fields are overwritten unconditionally, it means that embed fields are wiped when receiving a federated edit

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.

None yet

2 participants