Remove controller from the signatures of record URL functions?

[jimsafley]

@zerocrates claims that removing the $controller parameter from the signatures of Omeka_Record_AbstractRecord::getRecordUrl() and record_uri() should not cause undue problems. I am unconvinced because there are cases where the default controller is not the tableized class name (as with Plugin_IndexController) and actions names could be duplicated across controllers. Not sure what to do here.

[zerocrates]

My position is:

record_uri and getRecordUrl basically exist to guess the correct module/controller for a record, and to inject the right extra routing params, like id or slug.

1. In the case of "weird" records, like ones from plugins, this is exactly what getRecordUrl() is intended to do. A subclass' implementation knows what the correct controller and module should be, and what extra data (id, slug, whatever) to pass in the route.
2. In the case of multiple controllers that might have parallel actions, both dealing with the same record, I don't see this coming up that often, and, more importantly, I don't think allowing a user to pass the desired controller really helps. At that point, the function caller essentially knows exactly how they want to create the url, they're specifying the object, the action, and the controller. record_uri's really not doing much of substance at that point. All it's then handling is the extra params.

More simply, the $controller arg is only useful if the function caller knows what controller they want. Once they know that, there's not much that makes record_uri preferable to uri, so I don't think there's much point in designing record_uri to account for this situation.

[jimsafley]

Thanks for clarifying your position. My concern, then, is that the default controller is the inflected record type, e.g. "simple-pages-page", when it is often just "index". Isn't this something to consider, particularly when taking unpredictable plugin configurations into account?

[zerocrates]

You're right, the default controller would often be incorrect for particular records (and in that specific case you mentioned, a module would need to be provided too, probably). The record type inflection from the AbstractRecord is really only appropriate for built-in records, that have controllers within the default module.

But, this is exactly why the controller-guessing functionality was moved in to the AbstractRecord: so the non-standard records can take care of getting the routing params correct, including the controller, by specifying them in their own getRecordUrl() method. Before, record_uri just directly contained the controller-guessing code, so you really couldn't use if for plugin-added or non-standard records (unless, of course, you took care to specify the $controller param, though the module could then be an issue).

By handling it within the record class, you then don't have to vary your calls to link_to or record_uri just because this specific record handles things differently. You do have to override getRecordUrl(), though.

[jimsafley]

Understood. Ignoring edge cases, you've allayed my concerns. I'll remove controller at the appropriate places.