Fix follow IDs #455

Merged
elegaanz merged 4 commits from fix-follow-id into master 5 years ago
elegaanz commented 5 years ago (Migrated from github.com)

I changed the syntax of the insert! macro, allowing to modify an object just after it was saved to the database. You can now optional pass it a "closure" (actually it just looks like a closure) that will be run just after the insertion.

I updated all the models, including Follow, to use it to generate AP ID and URL. The advantages are that the ID/URL are now correct (fixes #449) and that we don't have methods like update_boxes or update_ap_url to call manually and that were easy to forget.

I changed the syntax of the `insert!` macro, allowing to modify an object just after it was saved to the database. You can now optional pass it a "closure" (actually it just looks like a closure) that will be run just after the insertion. I updated all the models, including Follow, to use it to generate AP ID and URL. The advantages are that the ID/URL are now correct (fixes #449) and that we don't have methods like `update_boxes` or `update_ap_url` to call manually and that were easy to forget.
elegaanz (Migrated from github.com) reviewed 5 years ago
@ -228,4 +228,3 @@
post.cover_id = form.cover;
post.update(&*conn, &searcher).expect("post::update: update error");;
let post = post.update_ap_url(&*conn).expect("post::update: update ap url error");
elegaanz (Migrated from github.com) commented 5 years ago

Just a note: I removed that without replacing it with something else, since it is incorrect (the AP IDs should change), and was not working anyway (since we were checking if ap_url was empty before updating it, which was only the case on newly created articles).

Just a note: I removed that without replacing it with something else, since it is incorrect (the AP IDs should change), and was not working anyway (since we were checking if `ap_url` was empty before updating it, which was only the case on newly created articles).
codecov[bot] commented 5 years ago (Migrated from github.com)

Codecov Report

Merging #455 into master will increase coverage by 1.41%.
The diff coverage is 79.31%.

@@            Coverage Diff             @@
##           master     #455      +/-   ##
==========================================
+ Coverage   25.06%   26.48%   +1.41%     
==========================================
  Files          64       64              
  Lines        6316     7111     +795     
==========================================
+ Hits         1583     1883     +300     
- Misses       4733     5228     +495
# [Codecov](https://codecov.io/gh/Plume-org/Plume/pull/455?src=pr&el=h1) Report > Merging [#455](https://codecov.io/gh/Plume-org/Plume/pull/455?src=pr&el=desc) into [master](https://codecov.io/gh/Plume-org/Plume/commit/a2b9d7ec44daf91d5b0dde00715d7185b5342c94?src=pr&el=desc) will **increase** coverage by `1.41%`. > The diff coverage is `79.31%`. ```diff @@ Coverage Diff @@ ## master #455 +/- ## ========================================== + Coverage 25.06% 26.48% +1.41% ========================================== Files 64 64 Lines 6316 7111 +795 ========================================== + Hits 1583 1883 +300 - Misses 4733 5228 +495 ```
trinity-1686a approved these changes 5 years ago
trinity-1686a left a comment
Owner

LGTM

LGTM
Owner

I've never used this syntax, but it looks super nice

I've never used this syntax, but it looks super nice
Owner

this route should really get implemented one day

this route should really get implemented one day
elegaanz (Migrated from github.com) reviewed 5 years ago
elegaanz (Migrated from github.com) commented 5 years ago

Yes, I think we already have an issue for it…

Yes, I think we already have an issue for it…
igalic (Migrated from github.com) reviewed 5 years ago
igalic (Migrated from github.com) commented 5 years ago

what does it do?

what does it do?
elegaanz (Migrated from github.com) reviewed 5 years ago
elegaanz (Migrated from github.com) commented 5 years ago

For all the fields that were not provided, it takes their value from the given struct (here NewBlog::default()).

For all the fields that were not provided, it takes their value from the given struct (here `NewBlog::default()`).

Reviewers

trinity-1686a approved these changes 5 years ago
The pull request has been merged as 2a188abfa1.
You can also view command line instructions.

Step 1:

From your project repository, check out a new branch and test the changes.
git checkout -b fix-follow-id master
git pull origin fix-follow-id

Step 2:

Merge the changes and update on Forgejo.
git checkout master
git merge --no-ff fix-follow-id
git push origin master
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: Plume/Plume#455
Loading…
There is no content yet.