Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RSS implementation #24

Merged
merged 12 commits into from May 19, 2014
Merged

RSS implementation #24

merged 12 commits into from May 19, 2014

Conversation

tmos
Copy link
Collaborator

@tmos tmos commented Feb 25, 2014

This RSS implementation use a database (in a flat file).

  • It generates a list of pictures, and store it
  • at the next generation, a diff is made, all new files are added at the rss feed.

To do :

  • RSS feeds for folders (probablyy add a regexp, if a folder is set on the url, recover just files fith folder's name on their adress).
  • Add a cache system : a little file with date of the last rss generation, and a user variable for the delay between two generations.

@tmos
Copy link
Collaborator Author

tmos commented Feb 25, 2014

Some points to remind, by Fred :

  • l.119 : for ($i=0; $i <= $nb_items_rss; $i++) { if the feed (temp)have less than25 items, it will bug,
  • Define $SkipExts and SkipObjects in config.php;
  • Use a locker to avoid a bug if rss.php is caled twice. Maybe the timestamp could be the locker?

@deejay87
Copy link

Good idea :)

@tmos
Copy link
Collaborator Author

tmos commented Apr 14, 2014

I use my own feed for few weeks now and I don't have any problems.
For now, here are the modifications that should be done :

  • l.119 : for ($i=0; $i <= $nb_items_rss; $i++) { if the feed (temp)have less than25 items, it will bug,
  • Define $SkipExts and SkipObjects in config.php;
  • Use a locker to avoid a bug if rss.php is caled twice. Maybe the timestamp could be the locker?
  • Use a regexp or a substring to set as title of the element the filename.

@deejay87
Copy link

I have found this good gallery but i have somes few problems , i try to look but it's difficult for me , i'm not a big coder ( i have open a new issue about this, but don't think lot of people come to try to find solution)
I have try the original who work more perfectly (but don't have some modifications done by this one)

What file we have to try for your rss ?
Thanks

@tmos
Copy link
Collaborator Author

tmos commented Apr 14, 2014

I don't recommend you to try the RSS feature at this moment, It is still not very clean. But if you really want to have a look, add the green parts of the "files changed" of this page.

@nodiscc
Copy link
Contributor

nodiscc commented Apr 23, 2014

Note: $gallery_link should be set to ./ by default as it works in all cases (no need to have the user configure it).

Edit: the RSS feed works properly for me. Thanks! I hope this can be merged soon

@tmos
Copy link
Collaborator Author

tmos commented Apr 28, 2014

Actually the gallery link is used to create the links in the RSS XML, so ./ is not really a good idea :-/

@nodiscc
Copy link
Contributor

nodiscc commented Apr 28, 2014

Indeed my XML entries look like

            </item><item>
                <title>.//photos/audiomz/8Ow7G.jpg</title>
                <link>.//photos/audiomz/8Ow7G.jpg</link>
                <description>
                    <![CDATA[ <img src=".//photos/audiomz/8Ow7G.jpg"> ]]>
                </description>
            </item><item>

but that doesn't prevent the feed from working. I've tried in Liferea and TinyTinyRSS and the feed displayed just fine, including images. How can this be a problem?

@tmos
Copy link
Collaborator Author

tmos commented Apr 28, 2014

Oh okay, I was wrong about it, sorry !
I'll change it as soon as I can, but if you want to wake a pull request, don't hesitate :)

nodiscc and others added 4 commits April 28, 2014 18:41
Addition to #24, change `$gallery_link` so it works everywhere by default, change `$description` to a generic description.
@tmos
Copy link
Collaborator Author

tmos commented Apr 30, 2014

to do :

  • l.119 : for ($i=0; $i <= $nb_items_rss; $i++) { if the feed (temp)have less than25 items, it will bug,
  • Use a locker to avoid a bug if rss.php is caled twice. Maybe the timestamp could be the locker?

@tmos
Copy link
Collaborator Author

tmos commented Apr 30, 2014

the link tag seems to don't work properly with the "./", can you confirm @nodiscc ?

@tmos
Copy link
Collaborator Author

tmos commented Apr 30, 2014

All seems to be okay, we just have to add :

  • the locker
  • the timestamp to avoid too much updates

I tink we can already merge as is seems already working pretty nice.

@nodiscc
Copy link
Contributor

nodiscc commented Apr 30, 2014

the link tag seems to don't work properly with the "./",

you mean the tag? I see no links in my rss feed, but the full image appears, see

cap

I think this is fine if there are no HTML link tags in the rss feed (should clicking on an image link to the image? or maybe i don't understand) Anyway http://you.minigal.adress won't work either, and will break the entire RSS feed, ./ is a saner default.

@tmos
Copy link
Collaborator Author

tmos commented May 1, 2014

The title have to be a link to the picture. The fake adress makes mandatory the edition and the user have to set a correct adress. ./ seems to be good in a first time but don't work for links, it's a trap and not a saner solution.

@nodiscc
Copy link
Contributor

nodiscc commented May 3, 2014

What's the purpose of having a clickable link to the image, when the full image is embedded in the RSS feed? Right-click > Display image is there for a reason.

The fake adress makes mandatory the edition

Adding one unnecessary step to the install process.

@tmos
Copy link
Collaborator Author

tmos commented May 3, 2014

I don't know if you noticed this, but every feed in the world have a link attribute, and as it name is «link», it should be a link to the item, even if it is just a picture.
I think adding one step to the admin is nothing compared to require all suscribers to Right-click > Display image.

@nodiscc
Copy link
Contributor

nodiscc commented May 3, 2014

RSS 2.0 specification:

[...] link and title may be omitted. All elements of an item are optional, however at least one of title or description must be present

Agreed, there is a small benefit to have clickable links (ability to view just the image in it's own browser tab/window, not in the rss reader). So the default is either ./ (broken links) or http://you.minigal.adress (RSS feed not working).

Maybe there is a way to retrieve the correct address automatically when building the feed, using dirname http://www.php.net/manual/fr/function.dirname.php (as rss.php is at the root directory of the gallery, something like $gallery_link = dirname($_SERVER['REQUEST_URI']) when loading rss.php, just a guess)

@tmos
Copy link
Collaborator Author

tmos commented May 4, 2014

Yep, this last solution could be great ! I'll have a look :)

@tmos
Copy link
Collaborator Author

tmos commented May 4, 2014

hey, I made it !
$gallery_link = substr("http://" . $_SERVER['HTTP_HOST'] . $_SERVER['REQUEST_URI'], 0, strlen("http://" . $_SERVER['HTTP_HOST'] . $_SERVER['REQUEST_URI'])-7); // the -7 is to remove "rss.php"
recover the base url, so the admin don't have to edit it, and the link tag is still working :)
With this, everybody is happy 👍
Tell me if it is working for you.
And thanks for your insistence…

@tmos
Copy link
Collaborator Author

tmos commented May 4, 2014

I set up the locker too.
RSS are now nearly finished, a last feature to add is the timestam.
It will avoid regenerating the RSS feed at each request. So users who update their gallery once a day can set-up the minimum update time at 24h for example

@tmos
Copy link
Collaborator Author

tmos commented May 4, 2014

Timestamp is done !
If some people could test this RSS branch it could be great :)
please tell me if it is okay or not, so I'll be able to merge in master in few days.

@nodiscc
Copy link
Contributor

nodiscc commented May 4, 2014

hey, good news. I see substr("http://", will this work over HTTPS? I can't test right now, but I guess not. Maybe there's a PHP function that returns the currently used protocol?

Also, before merging this, can you permanently remove the files deleted in https://github.com/tmos/MinigalNano/commit/9f27cdd56c4322cfc82791829fe0375136ac34b2 that are still in the git repo history? See https://github.com/nodiscc/scriptz/blob/master/dev/git/git-delete-history . Rewriting git history is bad but ... maybe better than merging db files.

Anyway thanks for your work ! 👍

@tmos
Copy link
Collaborator Author

tmos commented May 4, 2014

You are right for the HTTPS ! I'll add this as soon as possible !
I'll see for the history, it seems a little bit complex...

@nodiscc
Copy link
Contributor

nodiscc commented May 4, 2014

I think I have found a cleaner way to get a proper $gallery_link:

$gallery_protocol = (!empty($_SERVER['HTTPS']) && $_SERVER['HTTPS'] !== 'off' || $_SERVER['SERVER_PORT'] == 443) ? "https://" : "http://";
$gallery_domain = $_SERVER['HTTP_HOST'].'/';
$gallery_path = dirname($_SERVER['REQUEST_URI']);
$gallery_link = $gallery_protocol.$gallery_domain.$gallery_path

What do you think?

@tmos
Copy link
Collaborator Author

tmos commented May 5, 2014

Yes, it sounds great !
Can you test your code and do a pull request on my repo please ? I'll not be able to work a lot on minigal this week…
The "problem" may be to put non-editable content on the config.php, but I'm not sure about this. Tell me what you think about it :)

tmos added a commit that referenced this pull request May 19, 2014
@tmos tmos merged commit 2e1021c into sebsauvage:master May 19, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants