-
Notifications
You must be signed in to change notification settings - Fork 731
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
Legals #4660
Legals #4660
Conversation
@Suppress("UNCHECKED_CAST") | ||
((throwable.toRegistrationFlowResponse() | ||
?.params?.get(LoginFlowTypes.TERMS) as? JsonDict) | ||
?.get("policies") as? JsonDict) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bit dirty but it works...
@bmarty I think we can use this icon for it. |
private suspend fun getHomeserverTerms(baseUrl: String): GetTermsResponse { | ||
return try { | ||
executeRequest(null) { | ||
termsAPI.register(baseUrl + NetworkConstants.URI_API_PREFIX_PATH_R0 + "register") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this something we should consider adding to the spec? it looks like we're relying on the register
endpoint to fail
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the register endpoint will fail for sure, we are not providing any data to the request. Also if registration is disabled, I'm not sure what will happen.
This is a temporary workaround waiting for matrix-org/matrix-spec-proposals#3012 to land
/** | ||
* It's only possible to use this value for [getTerms] | ||
*/ | ||
Homeserver |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you think it's worth having a separate getHomeserverTerms
to avoid potential misunderstandings with this api?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I though about it, will iterate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
class ElementLegals @Inject constructor( | ||
private val stringProvider: StringProvider | ||
) { | ||
/** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small nit: I don't think this comment/doc adds much, do you think it's needed? renaming getData
to getServerPolicies
would avoid the need for a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably but a bit more confusing, since there is no server involved here...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
awesome stuff! 💯 hopefully this helps reduce our google play rejections
49721ed
to
bf7907a
Compare
Force-pushed to fix merge conflict in VectorPreferences |
Implement the first item of #4594
New
Legals
section in the main setting menu:@amshakal which icon can I used for this item?
EDIT: icon updated
Legals screen
Help and About screen updated
With a new help section. Fixes #4638