TheVilleOrg / phpbb-groupsub

This is a extension for phpBB3 that allows you to create paid subscriptions for members to gain access to usergroups.
https://www.phpbb.com/customise/db/extension/groupsub/
GNU General Public License v2.0
10 stars 11 forks source link

More Notifications #20

Closed Dark1z closed 4 years ago

Dark1z commented 4 years ago

There is a need for :

I hope you get me & understand it. Also I am hopping that this is not too much for you. If more clarification required then let me know.

Thanking you , Best regards πŸ‘

stevotvr commented 4 years ago

Added the first part: 430699b

Dark1z commented 4 years ago

Thanks 😍

First part working as it should, BUT I found that the Expire Notification doesn't get trigger when manually removed a subscription from a user.

By the above commit , there is a stevotvr.groupsub.notification.type.started in add_subscription function, BUT none for stevotvr.groupsub.notification.type.expired in end_subscription function.

Also one more thing , I do not know what behavior it should be... At : https://github.com/stevotvr/phpbb-groupsub/blob/1.2/operator/subscription.php#L373 and at : https://github.com/stevotvr/phpbb-groupsub/blob/1.2/operator/subscription.php#L479 You are removing the previous notifications , so should this stevotvr.groupsub.notification.type.started notification follow similar behavior ???

Best regards πŸ‘

stevotvr commented 4 years ago

First part working as it should, BUT I found that the Expire Notification doesn't get trigger when manually removed a subscription from a user.

I'll add a different notification for a cancelled subscription.

Also one more thing , I do not know what behavior it should be... At : https://github.com/stevotvr/phpbb-groupsub/blob/1.2/operator/subscription.php#L373 and at : https://github.com/stevotvr/phpbb-groupsub/blob/1.2/operator/subscription.php#L479 You are removing the previous notifications , so should this stevotvr.groupsub.notification.type.started notification follow similar behavior ???

The expired notification replaces the warning notification. The other notifications should stay in the user's notification history,

Dark1z commented 4 years ago

Ok. Do as you think is best.

Also i have some more improvements, especially on administrator side, to make it more user friendly & useful to admins. Should I make issues for them?

stevotvr commented 4 years ago

Yeah you should make a new issue if you have an idea.

I changed it so the warning and expire notifications will stay until the subscription is renewed. The notifications must be cleared before they can be sent again for the same item (subscription ID).

Dark1z commented 4 years ago

Ok.

Thanking you , Best regards πŸ‘

stevotvr commented 4 years ago

Added the second part mostly in 3715cc5 I still need to add the setting to toggle it. I might possibly create a custom permission or permissions for accessing different parts of the extension and receiving notifications.

Dark1z commented 4 years ago

Ok....

Best regards πŸ‘

stevotvr commented 4 years ago

I added the setting. I'll get to the custom permissions next.

Dark1z commented 4 years ago

Nice 😍

Best regards πŸ‘

Dark1z commented 4 years ago

I'll test this & let you know soon.... this will take some time coz it has migration changes & i am a bit busy ...

Thanking you , Best regards πŸ‘

Dark1z commented 4 years ago

@stevotvr ,

i just tested this with commit : b7827bf3a80e83f39f11c3195cfec50f84a4aafe & it works perfectly... Including the Migration FIX. Permissions are also awesome. 😍

Sorry for Delayed Response.

Best regards :+1:

stevotvr commented 4 years ago

Thanks for taking the time to test this.