Add support for generic timeline #525

trinity-1686a · 2019-04-11T18:39:24Z

fix #450
Query example : lang in [fr, en] and license in [cc] or followed or title contains "Plume"

todolist

Abstract syntax for query language

S -> A or S | A
A -> B and A | B
B -> ( S ) | C
C -> not D | D
D -> E in L | F contains W | G
E -> blog | author | license | tags | lang
F -> title | subtitle | content
G -> followed H | has_cover | local | all
H -> include I H | exclude I H | ε
I -> boosts | boost | likes | like
L -> [ M ] | W
M -> W, M | W
W -> <char+>

codecov · 2019-04-11T18:56:11Z

Codecov Report

Merging #525 into master will increase coverage by 5.48%.
The diff coverage is 69.63%.

@@            Coverage Diff             @@
##           master     #525      +/-   ##
==========================================
+ Coverage   34.35%   39.83%   +5.48%     
==========================================
  Files          68       72       +4     
  Lines        8104     9368    +1264     
  Branches     1923     2219     +296     
==========================================
+ Hits         2784     3732     +948     
- Misses       4538     4580      +42     
- Partials      782     1056     +274

also run migration before compiling, so schema.rs is up to date

still miss tests and query integration

and refactor other tests to use begin_transaction

trinity-1686a

at first glance this looks good. I'll run some tests when I get the time, to make sure all is ok

src/routes/timelines.rs

trinity-1686a

I think you are not calling the query parser when handling local like/reshare

plume-models/src/likes.rs

plume-models/src/reshares.rs

elegaanz · 2019-07-07T19:53:59Z

Thanks for the review, everything is fixed now.

But… I think posts are correctly removed from timelines when deleted, but not when un-liked or un-boosted (but this can probably be fixed later?).

trinity-1686a · 2019-07-07T20:01:05Z

they are remove because of cascading remove from sql. The easiest way to have the same comportment might be to add a reference to the like/reshare in 2 optional (and useless) columns, that cascade remove too

trinity-1686a

I can't approve this myself as it's my pr, so if someone could review it that would be nice
we should also do a little todolist of things yet missing.
What come to mind is :

boost/like deletion doesn't remove from tl
new tl are not automatically filled (make a magic sql query that does this?)
related to the above, modifying the query of a tl should clear the tl and rebuild it
having an actual way to modify a tl, else they are not really custom
I believe query parsing errors are not translated

src/routes/timelines.rs

elegaanz · 2019-07-23T17:35:08Z

I can't approve this myself as it's my pr, so if someone could review it that would be nice

I seems OK for me (excepted for the things you listed), but I worked on it too, maybe you want to review it, @igalic ? (don't feel forced to do it, I think we can safely merge it if someone else approves the few commits I pushed).

And for the issue you listed, maybe it could be handled in another PR ? The basics are here, and this branch is already pretty big…

igalic · 2019-07-23T17:46:06Z

I'll see what i can do tonight.

trinity-1686a · 2019-07-23T19:37:42Z

What I listed was just to have a small todolist of things to add later, not now. This should end in an issue to not get forgotten

igalic

weak 👀 very tired, not finished this review sorry

migrations/postgres/2019-06-24-101212_use_timelines_for_feed/up.sql

migrations/sqlite/2019-06-24-105533_use_timelines_for_feed/up.sql

plume-models/src/instance.rs

plume-models/src/lists.rs

igalic

slightly less tired, more comments, would love to hear your feedback

plume-models/src/blogs.rs

migrations/sqlite/2019-06-24-105533_use_timelines_for_feed/down.sql

build.rs

plume-models/src/lists.rs

plume-models/src/timeline/query.rs

plume-models/src/migrations.rs

plume-models/src/lists.rs

plume-models/src/timeline/mod.rs

igalic · 2019-07-26T22:09:49Z

plume-models/src/timeline/mod.rs

+                .map_err(Error::from)
+        } else {
+            timeline_definition::table
+                .filter(timeline_definition::user_id.is_null())


again, this duplication begs to be solved generically

changes since last review

trinity-1686a · 2019-09-13T09:54:29Z

and I failed my merge >_<

igalic

👍

igalic · 2019-09-13T10:16:34Z

plume-models/src/timeline/mod.rs

+                        }
+                    })
+            {
+                Err(err)?;


That is 21 lines of error handling

igalic · 2019-09-13T10:17:23Z

plume-models/src/timeline/mod.rs

+                        }
+                    })
+            {
+                Err(err)?;


that's another 21 lines of error handling

there should be a better way to do that …
(I'm not saying you have to find that now)

igalic · 2019-09-13T10:20:34Z

plume-models/src/timeline/mod.rs

+    /*
+    #[test]
+    fn test_matches_lists_saved() {
+        let r = &rockets();


why is this one commented out?

I have no idea

plume-models/src/timeline/mod.rs

Conflicts: plume-models/src/instance.rs src/routes/instance.rs

elegaanz

LGTM, this great PR have been waiting for too long!

Begin adding support for timeline

2112288

trinity-1686a added A: Front-End Related to the front-end C: Feature This is a new feature S: Incomplete This PR is not complete yet A: Backend Code running on the server labels Apr 11, 2019

trinity-1686a added 2 commits April 11, 2019 20:45

fix some bugs with parser

ff75bf4

fmt

9e7b060

add error reporting for parser

6a32146

trinity-1686a force-pushed the timeline branch from d0dee71 to 6a32146 Compare April 11, 2019 20:35

add tests for timeline query parser

9902893

trinity-1686a force-pushed the timeline branch 2 times, most recently from 051bb5c to f938ae6 Compare April 12, 2019 08:40

add rejection tests for parse

c009cba

trinity-1686a force-pushed the timeline branch 2 times, most recently from b89b07b to a01341d Compare April 13, 2019 20:38

begin adding support for lists

f55113f

also run migration before compiling, so schema.rs is up to date

trinity-1686a force-pushed the timeline branch from a01341d to f55113f Compare April 13, 2019 20:55

trinity-1686a added 4 commits April 14, 2019 13:15

add sqlite migration

4226295

end adding lists

9d8b4da

still miss tests and query integration

cargo fmt

5b2f046

try to add some tests

701ec1f

trinity-1686a force-pushed the timeline branch from 3278290 to def45dc Compare April 17, 2019 12:11

Add some constraint to db, and fix list test

2086ce7

and refactor other tests to use begin_transaction

trinity-1686a force-pushed the timeline branch from def45dc to 2086ce7 Compare April 17, 2019 13:21

add more tests for lists

51e5760

trinity-1686a force-pushed the timeline branch from 184735b to 51e5760 Compare April 17, 2019 14:19

trinity-1686a added 3 commits April 17, 2019 19:38

add support for lists in query executor

2e08029

Merge branch 'master' into timeline

7a92b4f

Merge branch 'master' into timeline

65c47c2

trinity-1686a commented Jun 27, 2019

View reviewed changes

src/routes/timelines.rs Show resolved Hide resolved

trinity-1686a commented Jul 7, 2019

View reviewed changes

plume-models/src/likes.rs Outdated Show resolved Hide resolved

plume-models/src/reshares.rs Outdated Show resolved Hide resolved

Use Kind::Like and Kind::Reshare when needed

e1a225a

Forgot to run cargo fmt once again

07d788b

trinity-1686a commented Jul 23, 2019

View reviewed changes

src/routes/timelines.rs Show resolved Hide resolved

trinity-1686a added 2 commits July 23, 2019 10:36

revert translations

fdd4132

Merge branch 'master' into timeline

e746b59

igalic reviewed Jul 25, 2019

View reviewed changes

igalic previously requested changes Jul 26, 2019

View reviewed changes

trinity-1686a added 2 commits July 28, 2019 18:15

fix reviewed stuff

98904c9

reduce code duplication by macros

12a952f

trinity-1686a requested a review from igalic August 13, 2019 10:34

cargo fmt

e03e3fa

trinity-1686a force-pushed the timeline branch from d225170 to e03e3fa Compare August 13, 2019 12:04

Merge branch 'master' into timeline

29d2815

Merge remote-tracking branch 'origin/master' into timeline

7e0f967

trinity-1686a force-pushed the timeline branch from be7f5a2 to 7e0f967 Compare September 13, 2019 10:03

igalic previously approved these changes Sep 13, 2019

View reviewed changes

Merge branch 'master' into timeline

bb84e20

Conflicts: plume-models/src/instance.rs src/routes/instance.rs

igalic dismissed their stale review via bb84e20 September 18, 2019 16:32

elegaanz approved these changes Oct 7, 2019

View reviewed changes

elegaanz merged commit 006b44f into master Oct 7, 2019

elegaanz deleted the timeline branch October 7, 2019 17:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for generic timeline #525

Add support for generic timeline #525

trinity-1686a commented Apr 11, 2019 •

edited by elegaanz

Loading

codecov bot commented Apr 11, 2019 •

edited

Loading

trinity-1686a left a comment

trinity-1686a left a comment

elegaanz commented Jul 7, 2019

trinity-1686a commented Jul 7, 2019

trinity-1686a left a comment

elegaanz commented Jul 23, 2019

igalic commented Jul 23, 2019

trinity-1686a commented Jul 23, 2019

igalic left a comment

igalic left a comment

igalic Jul 26, 2019

trinity-1686a commented Sep 13, 2019

igalic left a comment

igalic Sep 13, 2019

igalic Sep 13, 2019

igalic Sep 13, 2019

igalic Sep 13, 2019

trinity-1686a Sep 13, 2019

elegaanz left a comment

Add support for generic timeline #525

Add support for generic timeline #525

Conversation

trinity-1686a commented Apr 11, 2019 • edited by elegaanz Loading

todolist

Abstract syntax for query language

codecov bot commented Apr 11, 2019 • edited Loading

Codecov Report

trinity-1686a left a comment

Choose a reason for hiding this comment

trinity-1686a left a comment

Choose a reason for hiding this comment

elegaanz commented Jul 7, 2019

trinity-1686a commented Jul 7, 2019

trinity-1686a left a comment

Choose a reason for hiding this comment

elegaanz commented Jul 23, 2019

igalic commented Jul 23, 2019

trinity-1686a commented Jul 23, 2019

igalic left a comment

Choose a reason for hiding this comment

igalic left a comment

Choose a reason for hiding this comment

igalic Jul 26, 2019

Choose a reason for hiding this comment

trinity-1686a commented Sep 13, 2019

igalic left a comment

Choose a reason for hiding this comment

igalic Sep 13, 2019

Choose a reason for hiding this comment

igalic Sep 13, 2019

Choose a reason for hiding this comment

igalic Sep 13, 2019

Choose a reason for hiding this comment

igalic Sep 13, 2019

Choose a reason for hiding this comment

trinity-1686a Sep 13, 2019

Choose a reason for hiding this comment

elegaanz left a comment

Choose a reason for hiding this comment

trinity-1686a commented Apr 11, 2019 •

edited by elegaanz

Loading

codecov bot commented Apr 11, 2019 •

edited

Loading