New Module: favourites
|
alecmyers
Joined: 2006-08-01
Posts: 4342 |
Posted: Sun, 2007-11-18 15:27
|
|
Here's a new module that adds a dynamic album of user-selected favourites. Installation adds a new map to the database, and an ItemLinks item, "add to favourites" (which changes to "remove from favourites" when that item is already marked as a favourite. Please note there may be some cacheing issues at the browser with regard to this change of menu item.) As it stands, the add to favourites option is available for guest users too, and directs them to a message suggesting they register. It could be modified simply to remove the option for guests, but that wasn't what I wanted.[edit - this is now configurable on the admin page] As soon as a user has registered one favourite album or photo, a new SytemLink appears, "Favourites" which displays the dynamic album of Favourites. All comments and bug reports gratefully received. [edit] |
|
Posts: 32509
Great, I'm very happy you've implemented this!
I took a quick look at the code and here are some suggestions:
1. The database table should have a primary key and an index for columns that are used frequentely in where clauses.
- Please add <required/> in each <member> tag to add NOT NULL
- after the last </member>, add the following to add a primary key:
<key primary="true"> <member-name>userId</member-name> <member-name>itemId</member-name> </key>- Add <indexed/> in the <member> tag of itemId. Most queries are by userId or userId and itemId, thus I'd choose the primary key as {userId, itemId} which also serves as index for userId and userId,itemId. we only need to add a separate index for itemId alone for the clearFavourites($itemId) function and that's what is achieved by adding this <indexed/> tag.
- rename the table to American English FavoritesMap, that's our standard.
2. Mixing controllers and views: AddFavouriteView is used as a controller. If you need to perform some action / change persistent data, it should be a controller and not a view. The same applies to RemoveFavourite.
3. You can (and should) make it independent of the dynamicalbum module. any module can provide dynamic albums and modules should be independent. examples of modules that provide dynamic albums: dynamicalbum, keyalbum, rating, tags.
4. Missing permission check: in your getChildIds() method, you load all favorites of user X for the active user Y. anyone can see the favorites of other users with that design.
if you prefer that, that's fine. but you should perform a core.view permission check for all itemIds else the active user Y might get to see images that are not meant to be viewable for this user.
You can pretty much copy modules/rating/RatingAlbum.inc function getChildIds() (and its use of buildItemQuery()) to ensure that view permissions are respected
5. You don't need to package the GalleryStorage/tmp/ folder. You can leave that out.
6. I'd switch to American English everywhere. G2 is written in American English, on all levels (e.g. change AddFavouriteView to AddFavoriteView).
All but the actual URL view/controller names in the URL can then be localized to British English and other languages.
7. Your module should register an event listener for GalleryEntity::delete such that you can remove itemIds from the map once they get deleted.
8. Your addMapEntry() calls need to be protected with getMapEntry() to ensure that it doesn't attempt to add existing entries.
I'd fix the database table definition issues first. If you can fix them (and it's easy) in a few days, you won't have to provide module upgrade code yet since the module is so new.
Next on the list should be the item permission stuff, copying some code from RatingAlbum.
Thanks
--------------
Documentation: Support / Troubleshooting | Installation, Upgrade, Configuration and Usage
Posts: 4342
1. Will do
2. Is there any documentation on this? I appreciate the policy distinction between views and controllers - what's the practical difference? Also, is there an api for the green/red java boxes used for activating new modules? That would be a more appropriate way to handle this
3. If anyone has a mind to take this on that would be great. After three weeks of struggle, I still don't understand the Gallery api well enough for this.
4. Inasmuch as I understand you, I don't agree. Users can only see items they have 'core.ShowItem' on, but that's checked in UpdatesAlbumView's loadTemplate function. As for the userId passed to GetChildIds() - that's up to the loadTemplateFunction which calls $this->getChildIds($theme['actingUserId']) - I don't see how that can be subverted. I'm just using the same approach as the getChildIds from the other classes in DynamicAlbums - won't that do?
5. Fair enough
6. I can't possibly spell Favourite without the 'u' - it's just *wrong*
7. It does already
8. Fair enough
Posts: 32509
7. true, now i see it. perfect!
6. heh, ok.
4. nope, it's a real issue.
3. i'll see if i can find time next week to do this (and take care of #4 too).
2. It's the model view controller (MVC) design pattern.
) is handled by sending a request to conroller=favourites.AddFavourite&itemId=18, the controller's handleRequest method adds the favourite and then redirects to the view that displays the results (e.g. a message that says that the operation was successful, or the favourites album).
views should only be used to display data (load and prepare data for display).
controllers should be used when doing anything else but just displaying data. e.g. when changing data, use a controller.
a change request like adding a new favorite (sorry for my spelling
1. thanks!
--------------
Documentation: Support / Troubleshooting | Installation, Upgrade, Configuration and Usage
Posts: 4342
2. ok, I'll have another crack at this.
4. I'm sure you're right (ok, I'm being polite) but I followed exactly the model from the existing DynamicAlbums module and I really can't see a security hole: are you saying that users can have items displayed that they don't have core.ShowItem for? Or that users will be able to load others' favourites? Either way I can't see how, so please elaborate!
(if you're going to do number 3 that's great, but give me a few days to look at 2 and 4 first, but I'd appreciate more info on 4 so I can see the issue too.)
On spellings: I've put up with catalog instead of catalogue, color instead of colour, and disk instead of disc, for the 27 years since I first laid hands on an Apple ][ ... just think of it as a tiny token of revenge!
Posts: 32509
4. ok, you're right. one can't see the favorites of another user. so that's from the table.
but permissions can change and what once was visible to a user could now be forbidden. but since it's still in the favorites-map, the user will still be able to see the item.
that's a minor change.
--------------
Documentation: Support / Troubleshooting | Installation, Upgrade, Configuration and Usage
Posts: 4342
No they won't - because 'core.ShowItem' is checked on every item after entries have been fetched from the map but before anything is shown to the user. Of course that means they won't be able to remove the item from their favourites - but that's not a problem. It should stay as a favourite so that if they regain their permissions, it will once again appear as a favourite for them.
Posts: 32509
again, core.ShowItem isn't a permission.
and the core.ShowItem view only checks core.view permission when viewing a specific item. when viewing the album-page, you see a list of items (the favorites) and there's no other view involved to check permissions for you.
to be consistent with the rest of gallery, one can't show an item's title / summary / name if one doesn't have core.view access. even if one used to have core.view access for that item at some point.
--------------
Documentation: Support / Troubleshooting | Installation, Upgrade, Configuration and Usage
Posts: 4342
k, my bad, misreading the loadTemplate code in a hurry. I'll add a check for core.view permission on each item, probably in the helper class when the collection is loaded from the map.
Posts: 24
Man, I'm really happy you're working on this module. I've been wanting a "favorites" module for a while. Have you made anymore progress on it?
Posts: 4342
Aargh no, sorry, I'll get back into it this week. Thanks for letting me know that someone else will use it too!
Posts: 24
Absolutely! I'm very interested, and I have a bunch of users that are keen for it also. I'm really new at PHP - most of what I've learned, I've picked up from working with Wordpress and Gallery2 - but if there's any way I can help, I will. I'm looking forward to it!
Posts: 4342
Ok... this module is now updated.
Changes:
Still to do:
I think it will be straightforward to turn this into a Lightbox module for those who want one (please add a post on this thread if that's you) but it will be a fork of the code, my original use doesn't need that kind of complexity!
Please go carefully before installing this on a production server at this time, I'm sure there are some bugs. But if you want to try it and let me know what you can find that needs fixing, that would be great!
Also code reviews appreciated :whistle:
Posts: 4342
There's a glitch with the root album: it appears that when the root album is displayed, it calls getItemLinks() with an item which returns NULL from $item->getId(); this means that the root album always shows "Add to favourites" even when it is a favourite...
I'll look into this further, but for the meantime I've prevented the root album from being added in this module.inc
Posts: 32509
note: please update your first post of this topic. the latest version should always be the first download listed. else users end up with obsolete versions.
@root album:
$root->getId() returns the root album's id (usually 7, but it's not hardcoded).
$root->getParentId() returns null.
--------------
Documentation: Support / Troubleshooting | Installation, Upgrade, Configuration and Usage
Posts: 4342
If I update the first post, won't it drop to the bottom of the thread?
Posts: 32509
that only happens to replies, but not to the topic / "article." or in drupal-speak: you'll edit a node, not a comment.
Posts: 4342
I also forgot to say, this module doesn't depend on the dynamicalbums module any more, it's standalone.
The odd behaviour with the root album I think is caused by cache issues in the browser - for some reason after adding it to the favourites list (can't think why you'd want to, but it works anyway) the browser needs a cache reload (ctrl-f5) before it shows the menu item switching from "add to favourites" to "remove from favourites". That only seems to be the case with the root album though.
Posts: 27300
I think this module is great! Yet another "dynamic" album, cool.
Just a minor suggestion; define variables to prevent Notice errors:
&
Keep up the great work.
Dave
_____________________________________________
Blog & G2 || floridave - Gallery Team
Posts: 104
Another nice module Alec!
For some reason if I add a bunch of photos or folders to my favourites, and I go into the favourites folder I can only remove the first photo (the rest don't show the link). If I delete that one then I can delete the next one and so on, but there is no way for me to delete the others out of order. If I click on a photo from the favourites folder to goto it's individual page I can remove the photo, but not from the album view.
Anyone else experiencing this?
--
Chris
http://ChristianJamesPhoto.com
Posts: 4342
Version 0.2.1 for you all...
Chris that should be fixed now. And Dave, the undefined variables should be defined.
New in this version:
Still to do:
Please test and feed bugs back here.
Enjoy!
Posts: 104
Is there a way to differentiate between the "item actions" below items (photos, albums, links) and the item actions block that you add in the theme section?
Ideally I would like the item action "Remove XXX From Favourites" to show up under each item on the actual Favourites section; but not have "Add XXX To Favourites" under each item throughout my gallery; but at the same time to keep the link in the item actions block (for me at the bottom of each page)?
__________________________________________
Chris - http://ChristianJamesPhoto.com
Posts: 4342
Chris,
I don't that that's possible within the gallery structure - the Add/Remove item is just a regular ItemLink and will show up under every item for which it's valid, be that under the thumbnail for the item, or the larger image, according to where the theme normally displays ItemLinks. The code which decides whether it should be "Add...", "Remove..." (or neither) isn't provided with the context within which the ItemLinks are being sought.
If anyone knows different, let me know, but that's how I understand it to be - sorry.
(For myself I prefer themes without the itemLinks under thumbnails, I think they're terribly messy.)
Posts: 104
I ended up just removing all the ItemLinks under items except for when logged in as Admin. Actually the only one I really liked having under each album was your slideshow plugin link. Maybe I'll try and add it in on it's own as a link in the album.tpl
Also with the new version the "Offer Favourites to Guests" does not work on my gallery. When not logged in and trying to add an item to favourites it gives me a security violation error. If I downgrade to the previous version the "You're not logged in" page comes up just fine.
here is the error:
Security Violation
The action you attempted is not permitted.
Back to the Gallery
_________________________________
Chris - http://ChristianJamesPhoto.com
Posts: 4342
That's an easy hack then - Just add the slideshow link manually in your album.tpl template, under the code where each thumbnail is displayed, and leave the automatic ItemLinks turned off there.
Curious... and tricky, since you don't get a full error log as a guest.
Can you change this line in your config.php:
$gallery->setDebug('');to:
$gallery->setDebug('buffered');and send me (pm if you prefer) the debug output?
Posts: 32509
@guests:
in case you don't do that already...
change the getItemLinks code to only show the favorite links for users that are a member of the registered users group (and thus not for guests).
--------------
Documentation: Support / Troubleshooting | Installation, Upgrade, Configuration and Usage
Posts: 4342
Personally I *want* the link shown to guests (so when they use it they can be encouraged to join up) - but it's configurable in the admin page - that's the "Offer Favourites to Guests" that Chris mentioned.
Is it true that !registered_user => anonymous_user ?
Oh, I'm also working on localising (localizing?) the module so Favourites appears as "Favorites" (ick) for the benefit of our cousins.
Posts: 32509
> Is it true that !registered_user => anonymous_user ?
the guest user is a member of the everybody group but not a member of the registered users group.
just test with GalleryCoreApi::isUserInGroup() whether $gallery->getActiveUserId() is in the registered users group.
list ($ret, $registeredUsersGroupId) ) GalleryCoreApi::getPluginParameter('module', 'core', 'id.allUserGroup');
@localization:
you might not like it, but G2 is an en_US product. to localize it to en_GB (and it is localized to British English) one translates the few strings that need to be changed in the en_GB.po file.
and since it's a en_US product, it would be really weird if the module id of this module was in British English. it would be a singular exception, inconsistent.
i haven't made my mind up on this and such things need a wider discussion. after all, the module-id shouldn't matter too much. but it does. it's also part of the URLs (the prefix of the view and controller names). but is that so important?
i don't know about the module id, but you would do the community a great service if all strings in the module were en_US. else it doesn't really match the rest of the available plugins and the g2 framework assumes that the original strings are in en_US. even if there was a en_US.po /en_US.mo translation, it wouldn't get picked up because g2's hardcoded to en_US and won't even touch the gettext translation methods for en_US.
see:
http://codex.gallery2.org/Gallery2:Localization
--------------
Documentation: Support / Troubleshooting | Installation, Upgrade, Configuration and Usage
Posts: 4342
@localization:
Cough Cough... maybe an international product like Gallery2 should be such that it doesn't assume this?
When I can get make to produce a po file (it's consistently failing at the moment), I've no problem translating the strings back to US English (since I can then have them localized), but I'd rather have the URLs spelt correctly for my British customers (i.e. have the module name as favourites). And nobody else has bothered to write this module any time in the last four years.... ;-)
By the way, I've been following the info in that link, but as I say, I can't get the po file out:
Any suggestions?
Posts: 32509
> Cough Cough... maybe an international product like Gallery2 should be such that it doesn't assume this?
maybe you don't understand correctly.
g2 is fully internationalized. for the lack of a better word, it's prototyped in US English. all other languages or translations thereof.
> but I'd rather have the URLs spelt correctly for my British customers
if you're referring to the module-id / controller/view name:
the long-style URLs are indeed not internationalized. you can localize it by changing the short-URLs pattern though.
> (i.e. have the module name as favourites).
module name != module id. the module name as listed at site admin -> plugins is internationalized.
@po / mo files handling:
you need the developer package of g2. but you already seem to have that.
your "make en_GB.po" call looks fine.
> extract.php parse error: templates/FavouritesNotify.tpl:
i'd have to look at your FavouritesNotify.tpl file to see what causes that. i'll check that later.
--------------
Documentation: Support / Troubleshooting | Installation, Upgrade, Configuration and Usage
Posts: 4342
I do understand, I think - you said it's not possible to write a module whose original text is in anything other than US English, because Gallery will not touch an en_US.po for users whose preferred language setting is en_US. That's a great pity, and a disservice to module authors (and potential authors) whose first language is not American English.
If you could find the time to have a quick look at the failing .tpl that would be fab!
Posts: 32509
it's also an engineering problem. by ensuring that all plugins are available in en_US by default makes 90% of our users happy (the internet is mostly in (US) English).
we keep things consistent and guide plugin authors to do the right thing(TM).
on the other hand, we make it not too complicated to localize G2 to other languages. it can't be that complicated since G2 has been translated into many locales.
I agree that we could make the built-in language configurable and extract a en_US too. but it hasn't been requested to this point. maybe file a feature request and we'll see if the request gets some votes (gallery.menalt.com/sfvote).
Posts: 4342
The fact that the Internet is based around US English is a self-fulfilling defect that very many organisations are working hard to remedy. I'd prefer to think that Gallery supports translations not because it's easy, but because it's the "right thing to do" (trademark specifically *not* acknowleged).
And it's not very likely to be, because your constituency is preselected to be happy with en_US as a default. To be honest, it's a bit of a cop-out to answer every difficult question with "see if it gets enough votes" - there's much that happens in Gallery development that *doesn't* get voted for, according to the priorities of the core developers; I'm not saying this should be one of them, but to answer every suggestion with "see if it's popular" isn't particularly constructive.
Posts: 4342
I've found the issue with the .tpl file and making the .po:
{g->text text="...."}is accepted, while it barfs on{g->text text = "...."}- with the whitespace.So I should be ok to get this localised to US and GB English in a day or so.
Posts: 32509
I know, we both preferred to discuss the language stuff in a separate thread and if this website made it easy to split off discussions, I'd do it right away...but it's not possible without hacking the DB.
Hopefully my last statement on that sub-topic in this thread:
Although English isn't my first (or even second) language, I'm a proponent of keeping most of the internet in English. In a lot of places, English is either the first or second foreign language. Instead of a tower of babel, everyone understands everything. Communication happens around the globe. Information is freely available to everyone without major barriers.
Machine translation is just not good enough. If I want to read some Chinese websites, I only get a hunch of what they
re actually talking about. And I couldn't participate in any Chinese discussion.
@voting / popular / democracy:
Of course you're right. Not everything should be done even if it's popular and even if some things are not popular, they need to be done.
That's how it works.
We use sfvote mostly for features / requests that are not necessarily on our roadmap. And for the intersection of our roadmap / of things we deem important for the success of the project / software and popular requests, we use sfvote as an indication of what we should prioritize.
Do we only offer Gallery in US English or is it the only default? No, the default language is configurable and G2 even adjusts to all visitors' preferred language.
The level of flexibility you request is deeper, at the development level. It would be nice to have, but if you look at all the other things we need to work on, I can't prioritize this very high. This is where patches / sfvote / community development comes into play. If this is something someone needs and if the patch is reasonable, you're not depending on one of the few core developers to implement the feature. It can find it's way into G2's core module by 3rd party development with code reviews from the core team.
Anyhow, back to the favourites module.
--------------
Documentation: Support / Troubleshooting | Installation, Upgrade, Configuration and Usage
Posts: 4342
Now available, Favourites 0.2.2, with added transatlantic flavour. So you can choose your favourite favorite!
Posts: 4342
New version posted - 0.2.3. It has an improved admin page.
Posts: 244
Really great work - thanks for it.
I have integrated your current version (0.2.3) today.
Could you change line 210 of your module.inc from
to
.
With this change your first rewrite rule could be activated at the admin panel without modification.
Thanks for this gift.
Merry Christmas,
Jens
Posts: 4342
Yes, that's a typo. I still need to get the url rewrite functioning the way I want it to, though, so I think that block of code may get rewritten.
Glad you like the plugin otherwise!
Posts: 244
Yes, it's really fantastic! I was pleased to see, that you have realized it with SEO rules. I am in the process of optimizing it at my new site.
There is one thing I am not really understand. where is the difference between your second and your third rewrite rule? Isn't it a duplicate as I can use the %page% optionally?
__________________
http://jekophoto.de
Posts: 344
alec,
thanks for making this available! It seems to work great, except for some of the URL rewrites as you mentioned. Also, how does this affect duplicate content penalty? Now we have two copies of the same image/content (original) and (favorites).
Thoughts?
Posts: 4342
The intention of the three rewrite rules was to allow all three of the short-form urls to work:
.../favourites
.../favourites/<user-id>
.../favourites/<user-id>/<page>
However I gather that the Url Rewrite as implemented doesn't cater to short-forms that are subsets of other short forms, so it doesn't quite work out.
There's no penalty - the content isn't duplicated and the favourites "album" is dynamically created on request using the original items. The only extra storage is basically a list of favourites for each user that nominates them.
Posts: 30
Hi all!
Thanks a lot for this nice module. It's exactly what we needed.
..but..
I know that's problem on my site, but don't really know what I have to do for fix it. I installed this module, "Add.." and "Remove.." options are working properly, but if I want to go to "Favourites" dynamic album, it shows just blank page and browser says "Done" - so I think there's no bad script.
I checked it in Opera and IE as well.
Source code in blank page is just:
Thank you for any answer!
Posts: 4342
The script (Gallery) is terminating early, because of some error. Can you look at your php error log (your ISP's control panel will be the place to look for how to do this) and see if there's an entry that corresponds with the time that you get the blank page, please? Then post the details here.
Posts: 30
I think this is what you need:
Posts: 4342
Which version of the Favourites module do you have installed? The latest version is 0.2.3. The dependence on the dynamicalbum plugin was removed starting with version 0.2.0. You can download the latest version from the link in the first post in this thread.
Posts: 30
I've got 0.2.3 of course. I already tried to reinstall it, but it didn't help. I'm going to try it again, but....
EDIT: The reinstall didn't help.
Posts: 30
It's working now.. I'm sorry, my false.. I didn't have plugin called "Dynamic Album".
..but another questions..
1. How can I remove the link "Favourite" from the place, where is also "Logout" or "Your Account"
2. after 1... How can I place this link to the right-hand menu where is also "View Latest Comments" or "View Slideshow"
Thank you alecmyers for this very much!
Posts: 4342
There's a typo in one of the files that means the dynamicalbum module needs to be installed for this module to run- thanks for drawing my attention to this. I'll try to upload a fixed version later on. You should then be able to remove the dynamicalbum module.
I'm not entirely sure what areas of the page your two questions are asking about, as they depend on the theme that you're using. To change the Favourites link from being a Systemlink to (I guess) an ItemLink you'd need to rewrite the part of the module.inc file which define those links for this module.
Posts: 314
installed yesterday... great module!
also developed a small block that can be added on the photo/album pages, that displays the list of users that added that item to their favourites.
also a romanian .po translation.
new files attached in a zip; does not require modification to existing module files.
serbanc - www.e-poze.ro
Posts: 4342
Nice Idea!
I've updated the module to 2.2.4, deleting the line which (mistakenly) inserts a dependence on the dynamicalbum plugin, so you should no longer require that module to use this one.