store estimated reading time in database (#393) #1297
Conversation
|
Any idea what's happening with Travis CI ? |
|
I wonder if we should try looking for another solution. This method to estimate reading time is kind of really bad with non-Latin characters and/or some languages. |
|
What do you suggest? |
;-) |
|
@j0k3r I really don't understand why tests failed. Maybe I'm so tired. Go to bed. If you can help me ... ;-) |
|
Didn't find any real alternative, but it seems to me we could take in consideration pictures, with a time related to their size. |
|
Yeah, I found my error. Fixed. |
Seriously? It's an estimation of reading time, I don't think we should take in consideration pictures. |
|
Well, I thought it more like the time spent on the whole article. Weren't we considering counting the length of videos too ? |
| */ | ||
| public static function getReadingTime($text) | ||
| { | ||
| return floor(str_word_count(strip_tags($text)) / 200); |
There was a problem hiding this comment.
WP plugin seems to use 250 words per minutes (see here), maybe we can adjust it ?
Also, it could be interesting to explain the calculation, at least what is this number 200.
There was a problem hiding this comment.
Could you just add a comment about the 200 number? Telling that it is referencing to words per minute
|
I don't this a lot of tests in this PR :) |
|
Wikipedia shows some data about the number of words reads per minute : https://en.m.wikipedia.org/wiki/Reading_(process)#Reading_rate |
|
@modos189 I'll add filters to filter articles by estimated reading view. I'll add a select list (with 3 items "< 5min", "between 5 and 10" and "> 10 min") and 2 text fields "min" and "max" to allow the user to filter with personal filters. Next, we'll have a new filter, by domain name. |
|
|
|
I'm not sure that the filters icon is obvious for everyone. Thank you for your reactivity. |
|
It's normal when using material design, but on an desktop view it's taking time to access. |
|
Also, maybe buttons instead of a list to choose from (one click instead of two). |
|
|
The filter icon. You find it on any android app using material design. |
This PR is still in progress, don't worry ;-) |
|
|
|
I find this nice. |
|
👍 |
|
And we can add more filter options in the future. |
|
As for more filters (I write it here, I know it's dirty), there should be the date when the entry was created and the type (MIME type in database). EDIT : also author if we manage to have sufficient results |
nicosomb
force-pushed
the
v2-estimated-time
branch
from
ae1868f
to
019da6d
Compare
|
@j0k3r how do you think we can implement tests for this feature? Just by adding some articles (and setting estimating time by hand) and filling filter form? |
| @@ -32,6 +36,7 @@ public function addEntryAction(Request $request) | |||
|
|
|||
| $entry->setTitle($content->getTitle()); | |||
| $entry->setContent($content->getBody()); | |||
| $entry->setReadingTime(Tools::getReadingTime($content->getBody())); | |||
There was a problem hiding this comment.
Why don't you do that directly in the EntryEntity ?
| $entries = $this->getDoctrine() | ||
| $form = $this->get('form.factory')->create(new EntryFilterType()); | ||
|
|
||
| $filterBuilder = $this->getDoctrine() | ||
| ->getRepository('WallabagCoreBundle:Entry') | ||
| ->findUnreadByUser($this->getUser()->getId()); |
There was a problem hiding this comment.
Did you really need to call it find? getFilterForUnreadByUser doesn't work?
There was a problem hiding this comment.
It must start by findBy or findOne...
|
Just defined some fixtures (and/or updated existing one) and they will have estimated time. Then create a test for a filter by filling the form and check number of results. |
|
I pushed my draft for test. I need help for this one, if you can have a look... |
nicosomb
force-pushed
the
v2-estimated-time
branch
2 times, most recently
from
e405815
to
923a775
Compare
|
Ready for review. |
| ->orderBy('e.id', 'desc') | ||
| ->getQuery(); | ||
|
|
||
| $pagerAdapter = new DoctrineORMAdapter($qb); |
There was a problem hiding this comment.
Since you removed lines where DoctrineORMAdapter & PagerFanta were used, you can remove these use at the beginning of the file
There was a problem hiding this comment.
still used at the end of the file.
nicosomb
force-pushed
the
v2-estimated-time
branch
from
923a775
to
9a289fc
Compare
nicosomb
force-pushed
the
v2-estimated-time
branch
from
9a289fc
to
2686457
Compare
nicosomb
changed the title
|
finally good to merge? |
store estimated reading time in database (#393)
see #393