Adding unique ap_ids. Fixes #1100 #90

Merged
dessalines merged 14 commits from unique_ap_ids into activity-sender 1 year ago
Owner
There is no content yet.
dessalines reviewed 1 year ago
fn create(conn: &PgConnection, comment_form: &CommentForm) -> Result<Self, Error> {
println!("creating {}", &comment_form.ap_id);
// println!("creating {}", &comment_form.ap_id.as_ref().unwrap());
Poster
Owner

I had to comment this because it failed for some of the tests, where this ap_id is None.

I had to comment this because it failed for some of the tests, where this `ap_id` is None.
nutomic commented 1 year ago
Poster
Owner

Just put it into an if Some(ap_id) = form.ap_id?

Just put it into an `if Some(ap_id) = form.ap_id`?
dessalines reviewed 1 year ago
impl CommentForm {
pub fn get_ap_id(&self) -> Result<Url, ParseError> {
Url::parse(&self.ap_id)
Url::parse(&self.ap_id.as_ref().unwrap_or(&"not_a_url".to_string()))
Poster
Owner

Seemed easier than doing -> Option<Result<Url...

Seemed easier than doing `-> Option<Result<Url...`
nutomic reviewed 1 year ago
.on_conflict_do_nothing()
.on_conflict(ap_id)
.do_update()
.set(post_form)
nutomic commented 1 year ago
Poster
Owner

I did it like this on purpose, because otherwise someone could abuse a Create activity to update a post. And under normal circumstances, the old and new post should be identical.

I did it like this on purpose, because otherwise someone could abuse a Create activity to update a post. And under normal circumstances, the old and new post should be identical.
Poster
Owner

Hrm... I spose this is the case for comments too. I'll change both, either way they should get real updates through the push, not the fetcher anyway, and the push looks like it correctly uses ::update.

Hrm... I spose this is the case for comments too. I'll change both, either way they should get real updates through the push, not the fetcher anyway, and the push looks like it correctly uses `::update`.
Poster
Owner

Okay I finally got these builds working on travis: https://travis-ci.org/github/LemmyNet/lemmy/builds/722512931

Okay I finally got these builds working on travis: https://travis-ci.org/github/LemmyNet/lemmy/builds/722512931
Owner

Great! Tests are also passing fine for me (only tried one run). So this should be good to merge?

Great! Tests are also passing fine for me (only tried one run). So this should be good to merge?
dessalines merged commit 04a367b365 into activity-sender 1 year ago
The pull request has been merged as 04a367b365.
Sign in to join this conversation.
No reviewers
No Label
No Milestone
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

This pull request currently doesn't have any dependencies.

Loading…
There is no content yet.