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.

Sign up for GitHub

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

Jump to bottom

Adds Push Notification toggle to Device Manager #7261

Merged
merged 27 commits into from
Oct 10, 2022
Merged

Adds Push Notification toggle to Device Manager #7261

merged 27 commits into from
Oct 10, 2022

Conversation

ericdecanini
Copy link
Contributor

@ericdecanini ericdecanini commented Sep 29, 2022 •
edited
Loading

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other :

Content

Adds push notification toggle to device manager

Motivation and context

Epic: https://element-io.atlassian.net/browse/PSG-595
Closes: https://element-io.atlassian.net/browse/PSG-779

Screenshots / GIFs

Tests

  • Enable device manager feature flag in debug settings
  • Go to security settings and select Sessions v2
  • View details of a session and see that if there is a registered pusher of the device there is also a toggle

Tested devices

  • Physical
  • Emulator
  • OS version(s): Android 13

Checklist

@ericdecanini ericdecanini changed the base branch from develop to feature/eric/msc3881 September 29, 2022 18:50
@ericdecanini ericdecanini marked this pull request as ready for review September 29, 2022 18:51
@ericdecanini ericdecanini requested review from a team and onurays and removed request for a team September 29, 2022 18:53
onurays
onurays suggested changes Sep 30, 2022
View reviewed changes
Copy link
Contributor

@onurays onurays left a comment

Choose a reason for hiding this comment

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

Nice implementation! I have minor remarks. Please check the copyrights of all classes.

...d/src/test/java/org/matrix/android/sdk/internal/session/pushers/DefaultPushersServiceTest.kt Outdated
@@ -0,0 +1,66 @@
/*
* Copyright (c) 2022 New Vector Ltd
Copy link
Contributor

Choose a reason for hiding this comment

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

This need to be Matrix.org.

...src/test/java/org/matrix/android/sdk/internal/session/pushers/DefaultTogglePusherTaskTest.kt Outdated
@@ -0,0 +1,64 @@
/*
* Copyright (c) 2022 New Vector Ltd
Copy link
Contributor

Choose a reason for hiding this comment

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

Same remark.

matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fakes/FakeAddPusherTask.kt Outdated
@@ -0,0 +1,22 @@
/*
* Copyright (c) 2022 New Vector Ltd
Copy link
Contributor

Choose a reason for hiding this comment

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

Same remark.

matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fakes/FakeGetPushersTask.kt Outdated
@@ -0,0 +1,22 @@
/*
* Copyright (c) 2022 New Vector Ltd
Copy link
Contributor

Choose a reason for hiding this comment

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

Same remark.

...n/java/im/vector/app/features/settings/devices/v2/overview/SessionOverviewEntrySwitchView.kt
}

fun setOnCheckedChangeListener(listener: CompoundButton.OnCheckedChangeListener) {
binding.sessionsOverviewEntrySwitch.setOnCheckedChangeListener(listener)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should clear the listener onDetachedFromWindow() by setting as null.

...src/main/java/im/vector/app/features/settings/devices/v2/overview/SessionOverviewFragment.kt Outdated
isVisible = false
} else {
val allPushersAreEnabled = pushers.all { it.enabled }
setOnCheckedChangeListener { _, _ -> }
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand why this is needed. I think setting listener as null would seem cleaner setOnCheckedChangeListener(null).

@ericdecanini
Merge branch 'feature/eric/msc3881' into feature/eric/push-toggle-not…
225050d 
…ifications

# Conflicts:
#	vector/src/main/java/im/vector/app/features/settings/devices/v2/overview/SessionOverviewAction.kt
#	vector/src/main/java/im/vector/app/features/settings/devices/v2/overview/SessionOverviewFragment.kt
#	vector/src/main/java/im/vector/app/features/settings/devices/v2/overview/SessionOverviewViewModel.kt
#	vector/src/main/java/im/vector/app/features/settings/devices/v2/overview/SessionOverviewViewState.kt
#	vector/src/main/res/layout/fragment_session_overview.xml
#	vector/src/test/java/im/vector/app/features/settings/devices/v2/overview/SessionOverviewViewModelTest.kt
@ericdecanini
Fixes post merge errors
c038540
@ericdecanini
Merge branch 'feature/eric/msc3881' into feature/eric/push-toggle-not…
1639b71 
…ifications
onurays
onurays reviewed Oct 7, 2022
View reviewed changes
matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fixtures/PusherFixture.kt Outdated
@@ -0,0 +1,50 @@
/*
* Copyright (c) 2022 New Vector Ltd
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be Matrix.org.

onurays
onurays reviewed Oct 7, 2022
View reviewed changes
...ndroid/src/test/java/org/matrix/android/sdk/test/fakes/internal/FakePushGatewayNotifyTask.kt Outdated
@@ -0,0 +1,22 @@
/*
* Copyright (c) 2022 New Vector Ltd
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be Matrix.org.

onurays
onurays reviewed Oct 7, 2022
View reviewed changes
matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fakes/FakeTogglePusherTask.kt Outdated
@@ -0,0 +1,35 @@
/*
* Copyright (c) 2022 New Vector Ltd
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be Matrix.org.

onurays
onurays reviewed Oct 7, 2022
View reviewed changes
matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fakes/FakeTaskExecutor.kt Outdated
@@ -0,0 +1,25 @@
/*
* Copyright (c) 2022 New Vector Ltd
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be Matrix.org.

onurays
onurays reviewed Oct 7, 2022
View reviewed changes
matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fakes/FakeRemovePusherTask.kt Outdated
@@ -0,0 +1,22 @@
/*
* Copyright (c) 2022 New Vector Ltd
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be Matrix.org.

onurays
onurays approved these changes Oct 7, 2022
View reviewed changes
Copy link
Contributor

@onurays onurays left a comment

Choose a reason for hiding this comment

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

Please fix the remaining copyrights, otherwise LGTM!

Base automatically changed from feature/eric/msc3881 to develop October 10, 2022 16:37
@sonarcloud
Copy link

sonarcloud bot commented Oct 10, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

58.5% 58.5% Coverage
0.0% 0.0% Duplication

@ericdecanini ericdecanini merged commit 2fe636e into develop Oct 10, 2022
@ericdecanini ericdecanini deleted the feature/eric/push-toggle-notifications branch October 10, 2022 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@onurays onurays onurays approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ericdecanini @onurays