Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patching manage subscription #3423

Merged
merged 11 commits into from
Jul 11, 2019

Conversation

AntonioKL
Copy link
Contributor

Existing subscriber should be able to manage his setting without considering if someone is allowed to subscribe or not.

Fix a incident name issue
Remove slack unsubscribe
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
Existing subscription user should be able to manage his setting without considering if he is allowed to subscribe or not.
@jbrooksuk
Copy link
Member

@AntonioKL do you know if #3431 superseded this on purpose?

@AntonioKL
Copy link
Contributor Author

@AntonioKL do you know if #3431 superseded this on purpose?

Can you please explain? Not sure what do you mean?

@jbrooksuk
Copy link
Member

The linked PR has some of the same code as this branch?

@AntonioKL
Copy link
Contributor Author

The linked PR has some of the same code as this branch?

3431 is about GDRP , not sure how it is relevant for subscription management.

This is my own code and I didn't take it from anybody.

@jbrooksuk
Copy link
Member

The linked PR has some of the same code as this branch? ...

@AntonioKL sorry, I’m not suggesting you copied it, I was just wondering if there is overlap and the other PR addresses both issues? GDPR is closely related to subscriptions, you see.

@AntonioKL
Copy link
Contributor Author

The linked PR has some of the same code as this branch? ...

@AntonioKL sorry, I’m not suggesting you copied it, I was just wondering if there is overlap and the other PR addresses both issues? GDPR is closely related to subscriptions, you see.

Doesn't overlap.

@jbrooksuk
Copy link
Member

🤔 I just checked again and it doesn't. Sorry, I'm not sure what I'd looked at yesterday.

Before this gets merged, we still need to change the middleware, please.

@jbrooksuk
Copy link
Member

@AntonioKL ah, it's #3422 which has similar files.

@AntonioKL
Copy link
Contributor Author

@jbrooksuk
Please elaborate

@jbrooksuk jbrooksuk merged commit 163f78f into cachethq:2.4 Jul 11, 2019
@welcome
Copy link

welcome bot commented Jul 11, 2019

Hooray! Your first Pull Request was merged, here's to many more 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants