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

Make elgg_register_entity_url_handler a plugin hook instead #5290

Closed
beck24 opened this issue Mar 27, 2013 · 6 comments
Closed

Make elgg_register_entity_url_handler a plugin hook instead #5290

beck24 opened this issue Mar 27, 2013 · 6 comments

Comments

@beck24
Copy link
Member

beck24 commented Mar 27, 2013

"This reminds me of a thought I had a while back when I was working with entity urls - anytime you declare a url handler for an entity type it replaces the previous handler. So in a case like this where you just want to modify it in a specific case it can only happen in one plugin (well, I do have a workaround that can still work with the current API but it's not elegant). I think this would be better suited as a plugin hook where multiple plugins could have a chance of returning the url."

http://community.elgg.org/discussion/view/1200583/help-using-elgg-list-entities-or-alternate-solutions-for-my-problem

@Srokap
Copy link
Contributor

Srokap commented Mar 27, 2013

I don't like the idea of changing entity url depending on context, I think it should be rather responsibility of the view, not the mechanism registering it. This way we might cache response of getURL without worrying that it was called for the first time in wrong context or something.

However it reminds me of related idea I had. Often entities have their view, edit page, and delete link. Currently only first one is returned through method. How about passing optional parameter to ElggEntity::getURL() that would specify the type of the link. By default it would be view link, but we could specify other types (possibly different set of them). This way we could support handlers for other types, like edit or delete, but also for custom use cases, ie. entity url for particular context (it would be view's responsibility to ie. pass proper parameter to be used by icon/* view to fetch url with getURL).

@beck24
Copy link
Member Author

beck24 commented Mar 27, 2013

The only problem I see with that is like in the case specified in the thread, and in the case I had to deal with when I came up with context solution a while back - you just want the regular view from elgg_list_entities - or any other default view, with just a different url. If the view has to make that decision it means creating duplicate views with just a single variable difference - or changing all core views to use it through $vars.

I do like the sound of the second part of your idea though, having getURL() return a variety of entity urls depending on the parameter - edit/delete/icon/etc.

@Srokap
Copy link
Contributor

Srokap commented Mar 27, 2013

Yeah, additional logic in views has it's drawbacks.

I want to avoid here reaching too much to global state without any control. Elgg context are definitely useful, but I think they're pretty bad idea when it comes to optimizing stuff - we want to be context-free or at least have way of fabricating contexts (think of refreshing your view through ajax).

@beck24
Copy link
Member Author

beck24 commented Mar 27, 2013

excellent point - this is why I brought it up for discussion :)

@mrclay
Copy link
Member

mrclay commented Mar 28, 2013

👍 for moving this to hook.

@cash
Copy link
Contributor

cash commented Jun 2, 2013

URLs are handled by plugin hooks now. I used the icon url plugin hook as the pattern. Suggestions on better hook names welcome. See #5586

@cash cash closed this as completed Jun 2, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants