Hey, I would like to let my users send a newsletter but not create new newsletters types (terms).
I can set perm to send. But then users cant see Sent and Not sent newsletters lists.
So I need to enable administer newsletters perm.
Then is ok, but - List, add and edit newsletter series. - is also available.

How can I show Sent and Not sent lists to those who have perm to send without letting them creating and editing newsletter types ?

Thanks

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Simon Georges’s picture

Version: 6.x-1.1 » 6.x-2.x-dev

No new feature will be added to 6.x-1.x.
I suppose we could add a "list newsletter issues" permission.

Simon Georges’s picture

Version: 6.x-2.x-dev » 7.x-1.x-dev

Moving all feature request to 7.x.

amontero’s picture

Title: administer newsletters permission » Add 'administer simplenews categories' permission separate of 'administer newsletters'
FileSize
2.5 KB

This is because of 'administer newsletters' permission granting access to both:
- admin/content/simplenews -> The 'Newsletters' tab in the content overview page
- admin/config/services/simplenews -> The Configuration->Web services->Newsletters menu

I added a new 'administer simplenews categories' permission to check access to 2nd item. Now 'administer newsletters' permission only grants access to newsletters issues tab in content overview.

amontero’s picture

Component: Miscellaneous » Code
Status: Active » Needs review

Status: Needs review » Needs work
amontero’s picture

Status: Needs work » Needs review
FileSize
2.99 KB

Updated tests.

Status: Needs review » Needs work
amontero’s picture

Update tests, coding standards compliance.
Also added permission in commented code, 'just in case' anybody trips on it later :)

amontero’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work
amontero’s picture

Status: Needs work » Needs review
FileSize
5.89 KB

Update tests, coding standards compliance.

Status: Needs review » Needs work
amontero’s picture

Status: Needs work » Needs review
FileSize
7.78 KB

Update tests, coding standards compliance.

Status: Needs review » Needs work
Berdir’s picture

Status: Needs work » Needs review
Berdir’s picture

Thanks for working on this. The failing test is one that happens sometimes, just trigger a re-test and it should go away.

The problem is that Simplenews 7.x-1.x is now stable and with that comes the question what can be added to stable releases and what not. The additional t() strings aren' a problem, they're backend stuff and it's easy to update translations now using l.d.o.

However, what we probably should add is an update function that gives existing roles on existing sites the new permission if they have the old one, to avoid changing the module behavior there. Possibly with a short message informing the admin that he might want to uncheck those permissions.

Also, we need to check how this affects 7.x-2.x. We're changing the naming there, newsletter is actually a newsletter list/category and a single node is a newsletter issue. So a 7.x-2.x patch could possibly do the opposite of this one and add a separate permission for newsletter issues.

amontero’s picture

Hi, Berdir.
Thanks for your prompt response.
I've been banging my head for a good time trying to figure out what was wrong with the tests, good to know. Too bad I can't help with the test to sort what makes it fail sometimes.

Agree about update hook is needed. Will work on it.

As long as 7.x-2.x concerns, I think on naming permissions right at this point will ease upgrade path when 2.x becomes stable. However, changing permission naming in 1.x seems too dangerous, since it's stable.
Maybe a followup issue for 2.x among the upgrade path issues swapping just the permission names to allow room to 'administer simplenews categories' to exist in 2.x. Maybe it's against current nomenclature, but that would make site upgrade and contrib code upgrade to 2.x lots easier. I just mean machine naming. I'm not familiar with 2.x branch (just checked simplenews_permission() ), so it's just an idea I'm unsure of how can affect. What do you think?

Meanwhile how about such a message in the update hook?

t('A new permission has been added to simplenews module. You might want to review it on the <a href="@url">Permissions</a> page.', array('url' => url('admin/people/permissions')));
amontero’s picture

Working update hook.
Used functions in user.module safe at hook_update time, no external API calls other than DB involved.
Roles granted 'administer simplenews categories' permission are logged.
Hopefully nearing my first commit for simplenews :)

amontero’s picture

amontero’s picture

amontero’s picture

Bumping.
Is there anything else that should be taken care of to get this issue commited and closed?
TIA.

amontero’s picture

amontero’s picture

Bumping.
To all subscribers: as I've checked on IRC, this issue just needs someone to review and test it (including upgrade path) to be ready to be commited (RTBC). Anyone interested? TIA.