Always display identity server policies #4184

ouchadam · 2021-10-07T13:04:29Z

#4174 Always show identity server policies

Adds an expandable "Identity server policies" section in the Settings -> Discovery screen to view all terms/policies of the current identity server
Tapping a policy opens the policy url in an inapp chrome tab
Adds a "POLICY" link to the consent dialog, tapping takes the user to the discovery screen with the policies expanded
Updates the SettingsActivity navigation to allow sub screen payloads (eg expanding policies)

COLLAPSED	EXPANDED	POLICY DEEPLINK

… control the expansion

…s the policy urls view

…pandable form item

…policy is tapped

… non discovery screens

…ws us to have custom arguments for the sub settings pages

- also meant updating the general link to discovery in order to manually pass the default arguments

…licy link from the consent dialog

ouchadam · 2021-10-07T13:05:15Z

vector/src/main/java/im/vector/app/features/discovery/DiscoverySettingsViewModel.kt

-            is DiscoverySettingsAction.FinalizeBind3pid      -> finalizeBind3pid(action, true)
-            is DiscoverySettingsAction.SubmitMsisdnToken     -> submitMsisdnToken(action)
-            is DiscoverySettingsAction.CancelBinding         -> cancelBinding(action)
+            DiscoverySettingsAction.Refresh                        -> fetchContent()


might be worth revisiting this formatting rule as it causes noisy diffs 🤔

Yes, we had a debate about it... And noisy diff is THE inconvenient to keep aligned arrows. But with that we think the code is more readable...

github-actions · 2021-10-07T13:27:52Z

Unit Test Results

  42 files ±0   42 suites ±0 51s ⏱️ -1s
  83 tests ±0   83 ✔️ ±0 0 💤 ±0 0 ❌ ±0
220 runs ±0 220 ✔️ ±0 0 💤 ±0 0 ❌ ±0

Results for commit 5365e87. ± Comparison against base commit b71e2de.

♻️ This comment has been updated with latest results.

bmarty

Thanks for this PR.
Some (quite minor) remarks.
I do not always have strong opinion for all of them.
Also I'm wondering if we shouldn't make the same work for homeserver policies, but lets do it later.

vector/src/main/java/im/vector/app/features/discovery/DiscoverySettingsController.kt

vector/src/main/java/im/vector/app/features/discovery/DiscoverySettingsAction.kt

bmarty · 2021-10-07T13:31:48Z

vector/src/main/java/im/vector/app/features/discovery/DiscoverySettingsState.kt

@@ -21,10 +21,18 @@ import com.airbnb.mvrx.MvRxState
 import com.airbnb.mvrx.Uninitialized

 data class DiscoverySettingsState(
-        val identityServer: Async<String?> = Uninitialized,
+        val identityServer: Async<IdentityServerWithTerms?> = Uninitialized,


This is strange to me, maybe the Async should be removed here...

something to chat about tomorrow, I'm not familiar enough with the design to know why this async is unexpected

More info can be found here: https://airbnb.io/mavericks/#/async

bmarty · 2021-10-07T13:32:04Z

vector/src/main/java/im/vector/app/features/discovery/DiscoverySettingsState.kt

 ) : MvRxState
+
+data class IdentityServerWithTerms(
+        val serverUrl: String,


... and added here ...

bmarty · 2021-10-07T13:32:14Z

vector/src/main/java/im/vector/app/features/discovery/DiscoverySettingsState.kt

+
+data class IdentityServerWithTerms(
+        val serverUrl: String,
+        val policies: List<IdentityServerPolicy>


... and here

vector/src/main/java/im/vector/app/features/navigation/Navigator.kt

vector/src/main/res/values/strings.xml

vector/src/main/res/layout/item_discovery_policy.xml

bmarty · 2021-10-07T13:47:55Z

vector/src/main/res/layout/item_discovery_policy.xml

+
+    <!-- Do not use drawableEnd on the TextView because of RTL support -->
+    <ImageView
+        android:id="@+id/term_policy_arrow"


Minor: please rename this ID

You mean cb72609 :)

…te setting

bmarty

Thanks for the update, tested OK 👍

bmarty · 2021-10-07T18:59:11Z

(I've added a commit on develop with the changelog: b411938)

ouchadam · 2021-10-08T07:51:53Z

@bmarty thanks! still not used to added them 🤦

ouchadam added 13 commits October 6, 2021 15:20

adding the identity terms to the discovery page state

a2f6417

allowing the expandable scrollbar to be hidden

04d844b

binding the indentity server policy urls to the discovery page

7ce811c

binding the expanded state as part of the view, allows us to manually…

5a4f320

… control the expansion

updating discovery consent dialog to include policy link which expand…

3111d0b

…s the policy urls view

creating dedicated discovery policy items and displaying within an ex…

7e8ca29

…pandable form item

updating policy title to include show/hide

2f732af

using consistent naming for the policy and opening a chrome tab when …

c0fd266

…policy is tapped

navigating to the settings discovery page on policy link clicked from…

0516694

… non discovery screens

converting the settings activity payload to a sealed class, this allo…

728f34f

…ws us to have custom arguments for the sub settings pages

expanding the discovery polices on recieving a custom payload

7b5972e

- also meant updating the general link to discovery in order to manually pass the default arguments

launching the discovery page with policy expanded when tapping the po…

791c92c

…licy link from the consent dialog

reverting uneedeed text item changes

496a531

ouchadam commented Oct 7, 2021

View reviewed changes

bmarty added this to the Sprint - Element 1.3.2 milestone Oct 7, 2021

bmarty reviewed Oct 7, 2021

View reviewed changes

ouchadam added 5 commits October 7, 2021 17:29

using single action for the updating the policy expanded toggling/sta…

cccda9b

…te setting

handling empty policy list by showing empty copy

d00858f

using singular for the policy copy name to match content

259b6d5

renaming xml view to avoid clashing with other file

cb72609

reducing the discovery policy url text size by using caption style

5365e87

bmarty approved these changes Oct 7, 2021

View reviewed changes

bmarty merged commit d6af355 into develop Oct 7, 2021

bmarty deleted the feature/adm/is-policy branch October 7, 2021 18:53

bmarty added a commit that referenced this pull request Oct 7, 2021

Add changelog for #4184

b411938

ouchadam mentioned this pull request Oct 28, 2021

Link to contact detail sharing policy doesn't work #4356

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Always display identity server policies #4184

Always display identity server policies #4184

ouchadam commented Oct 7, 2021 •

edited

Loading

ouchadam Oct 7, 2021

bmarty Oct 7, 2021

github-actions bot commented Oct 7, 2021 •

edited

Loading

bmarty left a comment

bmarty Oct 7, 2021

ouchadam Oct 7, 2021

bmarty Oct 7, 2021

bmarty Oct 7, 2021

bmarty Oct 7, 2021

bmarty Oct 7, 2021

ouchadam Oct 7, 2021

bmarty Oct 7, 2021

bmarty left a comment

bmarty commented Oct 7, 2021

ouchadam commented Oct 8, 2021

Always display identity server policies #4184

Always display identity server policies #4184

Conversation

ouchadam commented Oct 7, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Oct 7, 2021 • edited Loading

Unit Test Results

bmarty left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bmarty left a comment

Choose a reason for hiding this comment

bmarty commented Oct 7, 2021

ouchadam commented Oct 8, 2021

ouchadam commented Oct 7, 2021 •

edited

Loading

github-actions bot commented Oct 7, 2021 •

edited

Loading