Fixing issues with Slack notifications, missing/wrong variables #3422
Conversation
Fix a incident name issue
Remove slack unsubscribe
|
Congratulations on opening your first Pull Request, this is a momentous day for you and us! ✨
|
Eding manage button for subscription
Adding manage route
enableSubscribers is used for Signup on the main dashboard, but cannot be forced for Admin on control
|
Adding feature to allow manage subscription out of admin GUI |
Existing subscription user should be able to manage his setting without considering if he is allowed to subscribe or not.
Manage subscribtion
| @@ -52,10 +52,6 @@ public function __construct(Repository $config) | |||
| */ | |||
| public function handle(Request $request, Closure $next) | |||
| { | |||
| if (!$this->config->get('setting.enable_subscribers')) { | |||
There was a problem hiding this comment.
Why have we removed this condition?
There was a problem hiding this comment.
Why have we removed this condition?
The subscription page must be manageable, despite the setting "Allow to subscribe". Admin should always be able to configure the subscribers and manage their subscriptions, etc.
You can not have this setting enabled or not based on "if the user is allowed to subscribe or not feature"
There was a problem hiding this comment.
Then the condition should check for an active admin user, then?
There was a problem hiding this comment.
It is done at the auth module ( outside of the scope for this commit)
There was a problem hiding this comment.
Why can't we add a condition into https://github.com/CachetHQ/Cachet/blob/2.4/app/Http/Middleware/SubscribersConfigured.php for admin?
There was a problem hiding this comment.
It is handled in app/Http/Kernel.php
@jbrooksuk
There was a problem hiding this comment.
If subscriptions aren't enabled and you're not an admin, then you shouldn't be able to see the subscription page, so as far as I can tell, the middleware is required.
|
@AntonioKL if you could address the middleware issue above, we could get this merged 😄 |
|
Please merge the code. Just tested the code and middleware in place |
|
Hello, Thx |
For now you have to manually update the |
|
Thank you for your input on Cachet 2.x. We are shifting our attention and resources to Cachet 3.x and will no longer be supporting the 2.x version. If your feedback or issue is relevant to the 3.x series, we encourage you to engage with the new branch. For more information on the Cachet rebuild and our plans for 3.x, you can read the announcement here. We appreciate your understanding and look forward to your contributions to the new version. |
No description provided.