JSON column for feeds by Alkarex · Pull Request #1838 · FreshRSS/FreshRSS

Alkarex · 2018-03-18T20:58:53Z

#1654

Alkarex · 2018-03-18T20:59:35Z

Lacks auto-create column for SQLite and PostgreSQL.

aledeg

I am not very keen of the column name but apart from that, good job!

Alkarex · 2018-03-18T21:24:01Z

@aledeg Any other name suggestion?

aledeg · 2018-03-19T06:25:31Z

I prefer to have the full name like attributes. We're not in the 80s when you needed to shorten everything to 8 chars :)

Alkarex · 2018-03-19T22:59:47Z

@aledeg What about json as a name, then? Short, and with additional information

aledeg · 2018-03-20T08:17:24Z

I think json is good, but it lacks content description. My 2¢.

Frenzie · 2018-03-20T08:32:27Z

I agree with @aledeg. json could be "anything." I have no real objection to something like attr, although I don't see why two characters more should count against attributes. :-)

Alkarex · 2018-03-21T08:51:27Z

This JSON column will actually be with anything inside :-)
So what do you vote for?

attribs
attributes
json
Yet another

I think I can now better like json, as it provides a bit more information, which we lack from the type (we cannot use the proper JSON types unfortunately due to poor compatibility)

Frenzie · 2018-03-21T09:38:01Z

json_attributes, jattr, j_attr, jsonAttributes
json_settings
json_properties

aledeg · 2018-03-21T11:01:07Z

jsonAttributes?

Alkarex · 2018-03-26T21:28:09Z

Ok, go for attributes before it gets even longer then :P

@aledeg Didn't you make the code to auto-add a column in PostgreSQL when you did the first version of the mute feed thing? I cannot find it anymore due to the squash inside the pull request. If you can find it again, I am interested :-)

aledeg · 2018-03-27T07:07:59Z

@Alkarex I am not sure I've done such thing. And if I did, I am afraid I've deleted it. Sorry

Alkarex · 2018-04-27T13:37:01Z

Tested with success in MySQL, SQLite, PostgreSQL

* Feed cURL timeout * Mark updated articles as read * Mark as read upon reception FreshRSS#1702 * Ignore SSL (unsafe) FreshRSS#1811

Alkarex · 2018-04-29T17:07:07Z

app/Controllers/feedController.php

 						// This entry should not be added considering configuration and date.
 						$oldGuids[] = $entry->guid();
 					} else {
+						$read_upon_reception = $feed->$attributes['read_upon_reception'] !== null


@Frenzie Do you know how those lines are supposed to be written to pass Travis / PHPCS?
I have already tried two variations without success...

Unfortunately it wants it all to be on one line like this:

diff --git a/app/Controllers/feedController.php b/app/Controllers/feedController.php index eb82a797..aa322a8b 100755 --- a/app/Controllers/feedController.php +++ b/app/Controllers/feedController.php @@ -353,9 +353,7 @@ class FreshRSS_feed_Controller extends Minz_ActionController { } else { //This entry already exists but has been updated //Minz_Log::debug('Entry with GUID `' . $entry->guid() . '` updated in feed ' . $feed->id() . //', old hash ' . $existingHash . ', new hash ' . $entry->hash()); - $mark_updated_article_unread = $feed->$attributes['mark_updated_article_unread'] !== null - ? $feed->$attributes['mark_updated_article_unread'] - : FreshRSS_Context::$user_conf->mark_updated_article_unread; + $mark_updated_article_unread = $feed->$attributes['mark_updated_article_unread'] !== null ? $feed->$attributes['mark_updated_article_unread'] : FreshRSS_Context::$user_conf->mark_updated_article_unread; $needFeedCacheRefresh = $mark_updated_article_unread; $entry->_isRead(FreshRSS_Context::$user_conf->mark_updated_article_unread ? false : null); //Change is_read according to policy. if (!$entryDAO->inTransaction()) { @@ -367,9 +365,7 @@ class FreshRSS_feed_Controller extends Minz_ActionController { // This entry should not be added considering configuration and date. $oldGuids[] = $entry->guid(); } else { - $read_upon_reception = $feed->$attributes['read_upon_reception'] !== null - ? $feed->$attributes['read_upon_reception'] - : FreshRSS_Context::$user_conf->mark_when['reception']; + $read_upon_reception = $feed->$attributes['read_upon_reception'] !== null ? $feed->$attributes['read_upon_reception'] : FreshRSS_Context::$user_conf->mark_when['reception']; if ($isNewFeed) { $id = min(time(), $entry_date) . uSecString(); $entry->_isRead($read_upon_reception);

So for legibility it might be better to use an if/else statement. I could also try to find out if there's a way to coax PHPCS into accepting this if you like but I don't know if that's an option.

Yes @Frenzie , please check if you can find a way to allow multiline ternary expressions

Alkarex · 2018-04-29T17:41:56Z

Feed cURL timeout
Mark updated articles as read Control of article updates per feed #891
Mark as read upon reception Mark as read when updating feeds #1702
Ignore SSL (unsafe) [Feature Request] Allow to ignore SSL certificate errors on a feed #1811

While waiting for a better syntax support

Frenzie · 2018-04-30T18:34:53Z

One workaround would be to simply remove the <rule ref="Squiz.WhiteSpace.OperatorSpacing"/> rule. Doing so will fail to detect incorrect spacing where we would want it to but it's not the biggest issue.

There are a few more targeted approaches:

diff --git a/phpcs.xml b/phpcs.xml
index d79e2a4d..1d10d277 100644
--- a/phpcs.xml
+++ b/phpcs.xml
@@ -92,7 +92,9 @@
        <!-- Do not add spaces when casting -->
        <rule ref="Squiz.WhiteSpace.CastSpacing"/>
        <!-- Operators must have a space around them -->
-       <rule ref="Squiz.WhiteSpace.OperatorSpacing"/>
+       <rule ref="Squiz.WhiteSpace.OperatorSpacing">
+               <exclude-pattern>./app/Controllers/feedController.php</exclude-pattern>
+       </rule>
        <!-- Do not add a whitespace before a semicolon -->
        <rule ref="Squiz.WhiteSpace.SemicolonSpacing"/>
        <!-- Do not add whitespace at start or end of a file or end of a line -->

If we updated to a newer PHPCS we could also selectively ignore rules for a few lines instead of for whole files (docs):

// phpcs:disable Generic.Commenting.Todo, PSR1.Files.SideEffects -- Because reasons
breaking_code();
// phpcs:enable Generic.Commenting.Todo, PSR1.Files.SideEffects

However, updating it would require some effort as well, also due to indentation issues.

That being said, in our current version we can still do this (i.e., selectively disable all checks):

diff --git a/app/Controllers/feedController.php b/app/Controllers/feedController.php
index eb82a797..4f478994 100755
--- a/app/Controllers/feedController.php
+++ b/app/Controllers/feedController.php
@@ -353,9 +353,11 @@ class FreshRSS_feed_Controller extends Minz_ActionController {
                                                } else {        //This entry already exists but has been updated
                                                        //Minz_Log::debug('Entry with GUID `' . $entry->guid() . '` updated in feed ' . $feed->id() .
                                                                //', old hash ' . $existingHash . ', new hash ' . $entry->hash());
+                                                       // @codingStandardsIgnoreStart
                                                        $mark_updated_article_unread = $feed->$attributes['mark_updated_article_unread'] !== null
                                                                ? $feed->$attributes['mark_updated_article_unread']
                                                                : FreshRSS_Context::$user_conf->mark_updated_article_unread;
+                                                       // @codingStandardsIgnoreEnd
                                                        $needFeedCacheRefresh = $mark_updated_article_unread;
                                                        $entry->_isRead(FreshRSS_Context::$user_conf->mark_updated_article_unread ? false : null);      //Change is_read according to policy.
                                                        if (!$entryDAO->inTransaction()) {

Alkarex · 2018-04-30T18:37:43Z

@Frenzie Thanks for the feedback and for trying to find a better rule. I think the current workaround is good enough for now, while waiting for a potential better approach later.
Let me know if you have a chance to test the new features of this PR :-)

Frenzie · 2018-04-30T19:06:04Z

@Alkarex Test in what sense exactly? Just some basic use it for a few days or actually trying to find where it might break? (The first is easier to squeeze in somewhere, naturally. :-P)

Alkarex · 2018-04-30T19:09:59Z

@Frenzie I only meant a quick look / run before merging, as further testing by more people can also be done once landed in /dev. I am already running this patch on my main instance.

Frenzie · 2018-04-30T19:18:37Z

I need to do some sewing right now but I should be able to do a basic test tomorrow. :-)

Frenzie · 2018-05-01T11:12:39Z

@Alkarex Nothing obviously broken so far. :-P

#1838 #891 #1702 #1811

FreshRSS#1838

* Feed attributes only for admin #1838 * Changelog 1905 #1905

* Draft of JSON column for feeds FreshRSS#1654 * Add some per-feed options * Feed cURL timeout * Mark updated articles as read FreshRSS#891 * Mark as read upon reception FreshRSS#1702 * Ignore SSL (unsafe) FreshRSS#1811 * Try PHPCS workaround While waiting for a better syntax support

FreshRSS#1838 FreshRSS#891 FreshRSS#1702 FreshRSS#1811

* Feed attributes only for admin FreshRSS#1838 * Changelog 1905 FreshRSS#1905

Should help with some DB lock issues. Complete FreshRSS#3558 after FreshRSS#5625 already cherry-picked from it. * Removed auto-update of MySQL GUID case sensitivity FreshRSS#2078 * Contributed to a DB lock in FreshRSS#5008 Also removed the following non-problematic auto-updates, simply because they were older than the above ones * Auto-create custom labels (1.12.0) FreshRSS#2027 * Auto-add JSON column for feeds (1.11.0) FreshRSS#1838 * Auto-create temporary tables (1.7.0) FreshRSS#1470

Should help with some DB lock issues. Complete #3558 after #5625 already cherry-picked from it. * Removed auto-update of MySQL GUID case sensitivity #2078 * Contributed to a DB lock in #5008 Also removed the following non-problematic auto-updates, simply because they were older than the above ones * Auto-create custom labels (1.12.0) #2027 * Auto-add JSON column for feeds (1.11.0) #1838 * Auto-create temporary tables (1.7.0) #1470

Draft of JSON column for feeds

afcb7bf

Alkarex added the Work in progress 🚧 Wait before merging label Mar 18, 2018

Alkarex added this to the 1.11.0 milestone Mar 18, 2018

aledeg reviewed Mar 18, 2018

View reviewed changes

Alkarex mentioned this pull request Mar 20, 2018

Feed is always marked as unread #1840

Closed

Rename attribs to attributes

77c9109

This was referenced Apr 8, 2018

Mark as read when updating feeds #1702

Closed

Per-feed option to strip query string (arguments) from article links #1865

Open

DB add JSON field #1654

Closed

[Feature Request] Allow to ignore SSL certificate errors on a feed #1811

Closed

Alkarex added 3 commits April 27, 2018 10:02

Merge branch 'FreshRSS/dev' into db-feed-json

5fcc003

More JSON feed attributes

da0c884

Fix addColumn

e462a43

Alkarex removed the Work in progress 🚧 Wait before merging label Apr 27, 2018

Alkarex added 2 commits April 27, 2018 16:09

Fix add feed

4ce4952

Add some per-feed options

a5daab7

* Feed cURL timeout * Mark updated articles as read * Mark as read upon reception FreshRSS#1702 * Ignore SSL (unsafe) FreshRSS#1811

Alkarex mentioned this pull request Apr 29, 2018

WIP: allow ignoring SSL certificate for some feed #1802

Closed

Travis PHPCS

06c22da

Alkarex commented Apr 29, 2018

View reviewed changes

Default message

166370d

Alkarex added 3 commits April 30, 2018 11:32

Try PHPCS workaround

40a608f

While waiting for a better syntax support

Syntax error

d63b894

Variable bug

bbca009

Alkarex mentioned this pull request Apr 30, 2018

Control of article updates per feed #891

Closed

Alkarex mentioned this pull request Apr 30, 2018

Impossible to subscribe to some RSS feed because of lack of HTTP headers #1627

Closed

Alkarex merged commit b552abb into FreshRSS:dev May 1, 2018

Alkarex deleted the db-feed-json branch May 1, 2018 15:02

Alkarex added a commit that referenced this pull request May 1, 2018

Changelog 1838

e1b30f0

#1838 #891 #1702 #1811

Alkarex mentioned this pull request May 28, 2018

FreshRSS 1.11.0 #1902

Merged

Alkarex added a commit to Alkarex/FreshRSS that referenced this pull request May 29, 2018

Feed attributes only for admin

59cc163

FreshRSS#1838

Alkarex mentioned this pull request May 29, 2018

Feed attributes only for admin #1905

Merged

Alkarex added a commit that referenced this pull request May 29, 2018

Feed attributes only for admin (#1905)

4ec1eba

* Feed attributes only for admin #1838 * Changelog 1905 #1905

mdemoss pushed a commit to mdemoss/FreshRSS that referenced this pull request Mar 25, 2021

Changelog 1838

dea497a

FreshRSS#1838 FreshRSS#891 FreshRSS#1702 FreshRSS#1811

mdemoss pushed a commit to mdemoss/FreshRSS that referenced this pull request Mar 25, 2021

Feed attributes only for admin (FreshRSS#1905)

7ebd995

* Feed attributes only for admin FreshRSS#1838 * Changelog 1905 FreshRSS#1905

Alkarex mentioned this pull request Sep 10, 2023

SQL: clean old auto-updates #5649

Merged

Uh oh!

Conversation

Alkarex commented Mar 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Alkarex commented Mar 18, 2018

Uh oh!

aledeg left a comment

Choose a reason for hiding this comment

Uh oh!

Alkarex commented Mar 18, 2018

Uh oh!

aledeg commented Mar 19, 2018

Uh oh!

Alkarex commented Mar 19, 2018

Uh oh!

aledeg commented Mar 20, 2018

Uh oh!

Frenzie commented Mar 20, 2018

Uh oh!

Alkarex commented Mar 21, 2018

Uh oh!

Frenzie commented Mar 21, 2018

Uh oh!

aledeg commented Mar 21, 2018

Uh oh!

Alkarex commented Mar 26, 2018

Uh oh!

aledeg commented Mar 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Alkarex commented Apr 27, 2018

Uh oh!

Alkarex Apr 29, 2018

Choose a reason for hiding this comment

Uh oh!

Frenzie Apr 29, 2018

Choose a reason for hiding this comment

Uh oh!

Alkarex Apr 30, 2018

Choose a reason for hiding this comment

Uh oh!

Alkarex commented Apr 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Frenzie commented Apr 30, 2018

Uh oh!

Alkarex commented Apr 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Frenzie commented Apr 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Alkarex commented Apr 30, 2018

Uh oh!

Frenzie commented Apr 30, 2018

Uh oh!

Frenzie commented May 1, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Alkarex commented Mar 18, 2018 •

edited

Loading

aledeg commented Mar 27, 2018 •

edited

Loading

Alkarex commented Apr 29, 2018 •

edited

Loading

Alkarex commented Apr 30, 2018 •

edited

Loading

Frenzie commented Apr 30, 2018 •

edited

Loading