scripting / feedBase

A project to get feeds into a base.
MIT License
33 stars 3 forks source link

Saturday work #8

Open scripting opened 6 years ago

scripting commented 6 years ago

Lots of cleanup work today and tomorrow.

But first I want to get the query for the user/subscriptions page.

That is, the page for a user showing what they're subscribed to.

Here's the page for myself.

http://feedbase.io/?username=davewiner

Note the undefined values in the third column.

image

This is where the countSubs value is supposed to be.

It's undefined because there is no countSubs field in the object returned for the subscription. I want to fix that.

http://feedbase.io/getsubs?username=davewiner

For each feed, countSubs should be the subscriber count for the feed across all users. The same value that appears in the hotlist.

http://feedbase.io/hotlist

Naively I tried taking the query for the hotlist, and hacked it up.

SELECT subscriptions.feedurl, feeds.title, COUNT(subscriptions.feedurl) AS countSubs FROM subscriptions, feeds WHERE subscriptions.feedurl = feeds.feedurl and subscriptions.username = "davewiner" GROUP BY feedurl ORDER BY countSubs DESC;

But the countSubs value for each was 1.

Any help much appreciated! ;-)

Dave

scripting commented 6 years ago

Just fixed up the feed viewer page.

http://feedbase.io/?feedurl=http%3A%2F%2Finessential.com%2Fxml%2Frss.xml

No longer displaying raw JSON.

The subscribers list is part of the table, and it's just a wrapping list of links.

Also encoding URLs in links.

More to come..

scripting commented 6 years ago

Making good progress. Now there's a Subscribe button on the Feed Viewer page.

image

In a sense this is the whole reason for having feedBase.

Dave

jystervinou commented 6 years ago

Hi,

Your request returns 1 for the count, because COUNT(subscriptions.feedurl) is indeed 1 when the data is restricted to subscriptions.username = "davewiner".

I see at least two ways:

You can do it in:

function addSubscriptionToDatabase (username, listname, feedurl, callback) {
    var now = formatDateTime (new Date ());
    var sqltext = "REPLACE INTO subscriptions (username, listname, feedurl, whenupdated) VALUES (" + encode (username) + ", " + encode (listname) + ", " + encode (feedurl) + ", " + encode (now) + ");";
    runSqltext (sqltext, function (result) {
                var getCountSQL = " SELECT count(*) AS c FROM subscriptions WHERE feedurl=" + encode(feedurl);
                runSqltext(updateCountsSQL, function(resultCount) {
                    var firstLine = resultCount[0];
                    var updateCountSQL = "UPDATE feeds SET countSubs = " + firstLine.c + " WHERE feedurl = " + encode(feedurl);
                    runSqltext(updateCountSQL, function(resultUpdate) {
              if (callback !== undefined) {
                  callback (result);
                  }
              });
                    });
                });
    }
scripting commented 6 years ago

First, thank you for the advice. I think it makes sense to do it this way, at least until another better way comes along.

I have made the change you recommend.

https://github.com/scripting/feedBase/blob/master/feedbase.js#L87

However I'm getting errors of this form:

runSqltext: err.code == ER_BAD_FIELD_ERROR, err.message == ER_BAD_FIELD_ERROR: Unknown column 'countSubs' in 'field list'

This seems to be happening in the third runSqlText call.

scripting commented 6 years ago

Wait I think I see what the problem is.

I have to recreate the database with a countSubs field in the subscriptions table.

Yes??

jystervinou commented 6 years ago

You need to create the new countSubs (or whatever name you prefer) column in the feeds table:

ALTER TABLE feeds ADD countSubs BIGINT;

jystervinou commented 6 years ago

No need to recreate the table, you can just ALTER it.

jystervinou commented 6 years ago

Or do it with a UI with the great Sequel Pro ;-)

scripting commented 6 years ago

Good we're making progress.

I added countSubs using the ALTER TABLE command from the command line.

The error messages went away.

Good.

But I think it's storing NULL in the database in the countSubs field.

Dave

scripting commented 6 years ago

Do you have a copy of the software running locally?

jystervinou commented 6 years ago

Nope, it's all running in my brain, which is not the greatest computer ever made :-D

That's probably the var firstLine = resultCount[0]; part that i got wrong.

jystervinou commented 6 years ago

try to log firstLine.c after firstLine = resultCount[0]; to check what's in there. (the .c is supposed to refer to the c in count(*) AS c

jystervinou commented 6 years ago

Sorry, watching at the same time ;-) [Update: lost at the last second :-/ ]

scripting commented 6 years ago

I took the time to rebuild the database anyway. it's populating now.

scripting commented 6 years ago

As it's rebuilding, it turns out some of them are not NULL.

http://feedbase.io/?username=davewiner

It's still reloading. I think perhaps NULL means "no one has set this value yet."

Which can be dealt with.

I'm going out for a bit myself, enjoy your sports game whatever it is.

Dave

jystervinou commented 6 years ago

Yes, NULL is the default value when no value has been set.

You can change that with:

ALTER TABLE feeds ADD countSubs BIGINT DEFAULT 0;

Or even enforce that a field should not be NULL:

ALTER TABLE feeds ADD countSubs BIGINT NOT NULL DEFAULT 0;

But then you loose the information that a field has not been set at all yet, compared to set to 0.

So better to check in code that the value === null.

scripting commented 6 years ago

It's obviously no big deal to check for that value before displaying it. ;-)

On Sat, Feb 3, 2018 at 2:17 PM, Jean-Yves Stervinou < notifications@github.com> wrote:

Yes, NULL is the default value when no value has been set.

You can change that with:

ALTER TABLE feeds ADD countSubs BIGINT DEFAULT 0;

Or even enforce that a field should not be NULL:

ALTER TABLE feeds ADD countSubs BIGINT NOT NULL DEFAULT 0;

But then you loose the information that a field has not been set at all yet, compared to set to 0.

So better to check in code that the value === null.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/scripting/feedBase/issues/8#issuecomment-362846451, or mute the thread https://github.com/notifications/unsubscribe-auth/ABm9O1zVkTk3remwZ7drjyk3BfYgaG60ks5tRLDlgaJpZM4R4Mlm .

jystervinou commented 6 years ago

What's weird is that you should not get any NULL any more on this page. All feeds have at least one subscriber (you).

Maybe addSubscriptionToDatabase is not called for all feeds when you rebuild?

scripting commented 6 years ago

I just uploaded v0.4.17.

The ctSubs column in the feeds table is being maintained, and is properly initialized for all feeds (that's the assertion).

You can see the column that was previously undefined is now full of information!

http://feedbase.io/?username=davewiner

Yes, I should (and will) sort the list on popularity. I just seems logical.

I also factored out a routine that takes the URL of a feed, and recomputes its ctSubs column. I wrote a routine that resets the value for all known feeds. I needed to do this at the end of the reload. It'll be nice to be able to do this.

scripting commented 6 years ago

Here are the changes in this release --

https://github.com/scripting/feedBase/commit/ed213de75054d3c58ff05840c07211482cc91112#diff-37ed9f574cf75afffb27a510967992f0

Dave

scripting commented 6 years ago

I've hit the stopping point for the day. Very good day of work. Thanks to JY as always.

So here's what's working.

  1. Every feed has a page.

  2. Every user has a page.

On the feed's page you get info about the feed, more to come, and a list of users who are subscribed. Click on their name to get to the user page.

On the user's page, we have a list of feeds they subscribe to, ranked by the number of subscribers. That works now. And when you click on the name of a feed, you go to the feed's page.

Every feed's page has a nice blue Subscribe button. Click it, and you get a dialog saying the feature is not yet implemented.

Checkboxes coming soon.

Done for the day! :rocket: