Base profile "My Classified Ads" list menu subitem on "create classified ads" permission.
b.hughes - June 16, 2007 - 23:45
| Project: | Classified Ads |
| Version: | 5.x-1.5-8 |
| Component: | User interface |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Michael Curry |
| Status: | postponed |
Description
Would it be possible to modify the code to not show the menu item "My Employment Ad list" based on the "create classified ads" access control? It seems to me that if you don't have permission to create ads, there's no point in seeing the menu.
Perhaps it should be viewable based on either create or edit own classified ads access.
#1
I may have been unclear when I stated "My Employment Ad", that's just the name of menu on my system. The content type for ed_classified is named "Employment Ad" on my site.
I also see that you are using user_access('access user profiles') to decide whether the menu item is visible. I've hack my own copy and changed it to user_access('create classified ads'), I'm making the assumption on my site that if a user has "create classified ads" permission they also have "edit own classified ads" and should see their own ads list.
#2
#3
The access control is for the currently logged in user, who is possibly viewing another user's ads. So, even if the user profile being viewed has no ability to create ads, the user profile being viewed may have ads owned by that user.
Therefore, using the current logged-in user's ability to create ads, or, the user whose profile is being examined, to determine visibility of the user profile ad list makes no sense to me.
The ad list is an attribute of a user profile, so it makes sense to allow anyone who can see a users' profile to be able to see any ads created under the target user account.
#4
There's another approach to the issue. Instead of enabling the link by permission, it could be enabled only if the user has ads to display. Obviously, if the user doesn't have permissions to create ads, they won't have any -- but even if they do have permission, and they haven't created any ads, there's no real reason to show the link.
Currently, this patch doesn't take into account whether the ad is published or not, but I noticed the list shows unpublished ads anyway. Is this by design, or should a user who doesn't have an "administer classifieds" permission not see unpublished ads?
#5
Thanks for the patch.
I've tried to point this out before - the purpose of this feature is to allow *other* users to see a user's classified ads.
How does this patch address that use case?
#6
Changing state
#7
Another way of looking at this issue is:
The "My classified ads" view is a subset, or specialization of, the 'tracker' functionality that lets site visitors having the appropriate permissions view users' posts.
Would you suggest that the tracker module limit functionality to allow only users having 'create node' permissions to see another user's posting list?
#8
I understand what you're saying, and I completely agree. With this patch, you should still be able to see other people's ads, even if you don't have permission to create them yourself. The patch doesn't work by permissions at all, it just sets the condition of showing the link on whether the user has created any ads. That way, you can still view other people's ads even if you don't have the permission to create them yourself. That way, a user who cannot create ads will inherently not have a link to "My Classified Ads." :)
The query which checks if a user has ads does not account for whether it's published or not. However, I've noticed that if I create ads with my admin account, and then look at it with a user who's permissions have only been altered to allow profile viewing, I can see a list of ads both published or not. This is probably something for which I should create a different issue, but it's kind of related in that the query does not determine whether to show the link based on the ad node's status. Of course, trying to follow a link to an unpublished ad with my dummy user leads to an access denied message. I was thinking about looking into creating a patch for that, but I'm not sure if there's a design decision behind it. :}
Anyway, this is the extend of what the patch does:
<?phpif (arg(0) == 'user' && is_numeric(arg(1))) {
$account = user_load(array('uid' => arg(1)));
if ($user !== FALSE && $account->uid) {
$has_ads = db_result(db_query("SELECT count(uid) FROM {node} WHERE type='%s' AND uid='%d'", EDI_CLASSIFIED_MODULE_NAME, $account->uid));
// only show the ads list in a user's profile if the user has ads
if (!empty($has_ads)) {
$items[] = array(
'path' => 'user/'.$account->uid.'/'.EDI_CLASSIFIED_PATH_NAME,
'title' => t('My @name list', $parms),
'callback' => 'ed_classified_by_user',
'callback arguments' => array(arg(1)),
'access' => user_access('access user profiles'),
'type' => MENU_LOCAL_TASK,
);
}
?>
#9
Ah, thanks. I had misread your original post and was unable to check out the patch. So, the patch suppresses the "My classified ads" tab and listing if the user has no ads published.
I'm not sure if this satisfies the original poster's concerns, but I like this approach.
Still, I think all of this is kind of unnecessary - as I mentioned before, if you look at the 'tracker' feature in the user's profile, nobody expects that tab to disappear if there are no posts for the user in question.
Why should it bother anyone that an empty list of ads appears under a user profile if there are in fact no ads created by that user? Why should the classified ads module behave differently than the tracker module?
(I hope I'm not sounding like I'm hostile to the idea; I'm just trying to figure out why the module behavior should change.)
#10
Well, my thought process is based on what I thought the original poster meant. When I first read this issue, I mistook the intent to be when viewing other people's profiles. It makes sense to me--if I'm viewing another user's profile, the link shouldn't show if that user doesn't have permission to create ads. It might be confusing if said user is specifically not allowed to create an ad for whatever reason. So, I thought of making the link appear only if the user whose profile you're viewing has permission to create ads. Then, it occurred to me that if you revoked the permission, but you still wanted the remaining ads live out their duration (subscription-based perhaps?), it would be better to keep that list until the current ads expired. Thus, I thought if the link were hidden from anybody who doesn't have any ads, it might save some confusion, particularly for users who might not have that privilege. I don't believe it necessarily hurts anything otherwise, but it might save a superfluous click here and there. In that, it seems to me that the classified ads module is different than the tracker module because it lists only one specific node type, whereas the tracker module lists any node type. :)
#11
@lbh:
I understand what you are getting at, and it makes sense from one perspective.
But what if the feature serves as a way of saying "This user has no classified ads at this time" ? I think that's a valid bit of information for site visitors.
The OP's suggestion does make some sense, and has some merit - one problem raised by the original idea is: what happens if the user had permission to create ads, and in fact created some, then, the admin revokes said permissions for that user: now the user has ads, but no longer has permission to edit his/her own ads or create new ones. The profile tab is still relevant and needed, but the OP's suggested feature change would now hide the profile tab.
In any case, I'm not hostile to suppressing this tab in the user profile based on some criteria; I am trying to clarify the use cases and ensure that everyone understands that things are a bit more complicated than it seems. In particular, I think that if I implement either the OP's suggestion, or yours, then other people who use this module will raise another issue because it doesn't behave the way they want.
If I decide to implement a feature along these lines ("Suppress My Classified Ads in User Profile if user cannot create ads" or whatever I decide to call it) then it will have to be able to satisfy the OP, you, and everybody else who uses or may want to use the module. Perhaps it should be an administrator option and/or a role-based permission setting...
Perhaps this 'feature' should be triggered if, and only if all the following conditions are true:
And even then, I'm not sure this would satisfy everyone... thus triggering other issue requests. At least that's been my experience. :)
Again, thanks for all your input and time thinking about this (and your other submissions). I'll try to make some time to review your other recent submissions and provide feedback.
-Mike
#12
@lbh:
Consider:
This module uses the published/unpublished status (along with expiration date) as a simple method of managing ad expiration. So, when an ad is expired based on the date, the ad is not deleted immediately. It's kept around for a while in order to allow the administrator to renew the ad (reset the expiration date.)
So, the Administrator and the user who has 'edit own classified ads' and 'reset classified ad expiration' permissions should be able to renew an expired ad by clicking the 'edit' link and checking the 'renew' box, or make a request that the administrator renew the ad. If this feature is broken, then we should open another issue so that I can investigate and/or fix it.
This is why the user profile feature shows unpublished ads in the list. I'll admit that the current implementation may be sub-optimal or broken, so it merits its own discussion. So, if that's the case, let's move this to a separate issue.
#13
Yeah, I understand where you're coming from; it's never quite as simple as it seems. The idea of having an option in the admin panel was something I actually considered suggesting. Perhaps I'll tinker around and create a patch for you to review. I really don't mind contributing, it's a good way help out and learn the "nuts and bolts" at the same time. . . I appreciate your module as both something that's useful for and as a learning tool. Hopefully you'll find my other patches useful--I'm actually running my test site on postgres. :)
I'll work on the list page itself too, and create a separate issue for it when I'm actually getting somewhere.
#14
No activity on this thread, so.. postponed.
#15
I applied the patch mentioned in #4 above and also modified the user access as described in #1 above.
The situation I had was that the user created a classified ad but logged out before paying for it.
When user logs back in, there's no way to find his ad to then pay for it.
Applying these fixes solved the problem for me - thanks!
Drupal 5.10
ed_classified 5.x-1.5-8
#16
subcribing