Skip to content
This repository was archived by the owner on Aug 9, 2021. It is now read-only.

WIP Disable notifications policy#430

Open
DIOHz0r wants to merge 1 commit into
flyve-mdm:developfrom
DIOHz0r:feature/405_disable_notifications
Open

WIP Disable notifications policy#430
DIOHz0r wants to merge 1 commit into
flyve-mdm:developfrom
DIOHz0r:feature/405_disable_notifications

Conversation

@DIOHz0r
Copy link
Copy Markdown
Contributor

@DIOHz0r DIOHz0r commented Apr 13, 2018 •
edited by btry
Loading

Signed-off-by: Domingo Oropeza doropeza@teclib.com

Changes description

Added the policy for disable all notifications on device

Checklist

Please check if your PR fulfills the following specifications:

  • Tests for the changes have been added
  • Docs have been added/updated

References

Closes #405
Depends #https://github.com/flyve-mdm/android-mdm-agent/issues/371

accesslint[bot]
accesslint Bot previously approved these changes Apr 13, 2018
View reviewed changes
@DIOHz0r DIOHz0r force-pushed the feature/405_disable_notifications branch from 63299e3 to 6bdce32 Compare April 13, 2018 15:28
@btry btry self-requested a review April 20, 2018 14:38
btry
btry suggested changes Apr 20, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@btry btry left a comment •
edited by ajsb85
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking for now, same comment as #431

@ajsb85 ajsb85 added the feature label Apr 21, 2018
@ajsb85 ajsb85 added this to the 2.0 milestone Apr 21, 2018
ajsb85
ajsb85 previously approved these changes Apr 21, 2018
View reviewed changes
ajsb85
ajsb85 reviewed Apr 21, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@ajsb85 ajsb85 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ToDo: the ability to "explode" this policy info more fine tunable set of policies.

@ajsb85 ajsb85 requested a review from rafaelje April 23, 2018 22:31
@ajsb85 ajsb85 changed the title Disable notifications policy WIP Disable notifications policy Apr 25, 2018
@btry btry mentioned this pull request Apr 29, 2018
Merged
@DIOHz0r DIOHz0r force-pushed the feature/405_disable_notifications branch 2 times, most recently from 321b07c to 999fbc4 Compare April 30, 2018 21:32
@ajsb85 ajsb85 requested a review from Naylin15 May 22, 2018 10:32
@DIOHz0r DIOHz0r force-pushed the feature/405_disable_notifications branch 3 times, most recently from 0db6470 to 351d77b Compare May 31, 2018 15:20
@DIOHz0r DIOHz0r force-pushed the feature/405_disable_notifications branch 2 times, most recently from 8f9099c to 6f04940 Compare July 10, 2018 14:13
@DIOHz0r
Copy link
Copy Markdown
Contributor Author

DIOHz0r commented Jul 10, 2018

@rafaelje, does this policy has been implemented on the current agent version?. I think this is a 2.1 plugin milestone and not 2.0

@btry
Copy link
Copy Markdown
Contributor

btry commented Jul 10, 2018

@DIOHz0r Also, this string areNotificationsEnabled is not uniform with other policies.

I think disableNotifications is better for consistency with other policies.

Can we also split this policy into several policies for different notification types ?

@DIOHz0r
Copy link
Copy Markdown
Contributor Author

DIOHz0r commented Jul 10, 2018

As far as I know there is no more options for notifications right now, could you confirm that @rafaelje ?

@DIOHz0r DIOHz0r force-pushed the feature/405_disable_notifications branch 2 times, most recently from ba3b040 to 4c1c443 Compare July 19, 2018 13:49
@DIOHz0r DIOHz0r force-pushed the feature/405_disable_notifications branch from 4c1c443 to b69d626 Compare July 20, 2018 20:07
@DIOHz0r DIOHz0r force-pushed the feature/405_disable_notifications branch from b69d626 to 205e73f Compare August 21, 2018 15:20
@DIOHz0r
Copy link
Copy Markdown
Contributor Author

DIOHz0r commented Aug 30, 2018

ping @rafaelje

@rafaelje
Copy link
Copy Markdown

rafaelje commented Aug 31, 2018

This policy is not implemented on the agent yet is for 2.1

@btry btry modified the milestones: 2.0, 2.1 Aug 31, 2018
@btry
Copy link
Copy Markdown
Contributor

btry commented Aug 31, 2018

@DIOHz0r please change the symbol of this policy, look at my old comments

@DIOHz0r
feat(policy): disable all notifications on device
75a2bde 
Signed-off-by: Domingo Oropeza <doropeza@teclib.com>
@DIOHz0r DIOHz0r force-pushed the feature/405_disable_notifications branch from 205e73f to 75a2bde Compare August 31, 2018 15:26
@DIOHz0r DIOHz0r requested review from btry and removed request for Naylin15 August 31, 2018 15:27
btry
btry approved these changes Aug 31, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@btry btry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, but we need to wait implementation in the agent before merge.

Added dependency to the issue in the related project.

@ajsb85 ajsb85 requested review from Naylin15 and ajsb85 September 21, 2018 09:15
@ajsb85 ajsb85 closed this Dec 18, 2018
@ajsb85 ajsb85 reopened this Dec 18, 2018
@ajsb85 ajsb85 changed the base branch from develop to features/for_later December 18, 2018 00:25
@btry btry closed this Dec 20, 2018
@btry btry changed the base branch from features/for_later to develop December 20, 2018 17:44
@btry btry reopened this Dec 20, 2018
@ajsb85 ajsb85 assigned DIOHz0r and unassigned DIOHz0r Dec 21, 2018
@ajsb85 ajsb85 modified the milestones: 2.1, 3.0 Aug 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@rafaelje rafaelje Awaiting requested review from rafaelje

@ajsb85 ajsb85 Awaiting requested review from ajsb85

@Naylin15 Naylin15 Awaiting requested review from Naylin15

2 more reviewers

@btry btry btry approved these changes

@accesslint accesslint[bot] accesslint[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@DIOHz0r DIOHz0r

Labels

feature

Projects

None yet

Milestone

3.0

Development

Successfully merging this pull request may close these issues.

Disable all notifications policy

5 participants

@DIOHz0r @btry @rafaelje @ajsb85 @Naylin15