Merge pull request #3030 from element-hq/feature/bma/codeCleanup

Code cleanup and store badges
This commit is contained in:
Benoit Marty
2024-06-14 15:15:01 +02:00
committed by GitHub
24 changed files with 160 additions and 208 deletions

View File

@@ -33,6 +33,27 @@ jobs:
- name: Search for invalid screenshot files
run: ./tools/test/checkInvalidScreenshots.py
checkDependencies:
name: Search for invalid dependencies
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Use JDK 17
uses: actions/setup-java@v4
with:
distribution: 'temurin' # See 'Supported distributions' for available options
java-version: '17'
- name: Configure gradle
uses: gradle/actions/setup-gradle@v3
with:
cache-read-only: ${{ github.ref != 'refs/heads/develop' }}
- name: Set up Python 3.12
uses: actions/setup-python@v5
with:
python-version: 3.12
- name: Search for invalid dependencies
run: ./tools/dependencies/checkDependencies.py
# Code checks
konsist:
name: Konsist tests

View File

@@ -13,6 +13,7 @@
<w>onboarding</w>
<w>placeables</w>
<w>posthog</w>
<w>rageshake</w>
<w>securebackup</w>
<w>showkase</w>
<w>snackbar</w>

View File

@@ -14,6 +14,8 @@ The application is a total rewrite of [Element-Android](https://github.com/eleme
Learn more about why we are building Element X in our blog post: [https://element.io/blog/element-x-experience-the-future-of-element/](https://element.io/blog/element-x-experience-the-future-of-element/).
[<img src="https://play.google.com/intl/en_us/badges/static/images/badges/en_badge_web_generic.png" alt="Get it on Google Play" height="80">](https://play.google.com/store/apps/details?id=io.element.android.x)[<img src="https://fdroid.gitlab.io/artwork/badge/get-it-on.png" alt="Get it on F-Droid" height="80">](https://f-droid.org/packages/io.element.android.x)
## Table of contents
<!--- TOC -->
@@ -25,13 +27,13 @@ Learn more about why we are building Element X in our blog post: [https://elemen
* [Contributing](#contributing)
* [Build instructions](#build-instructions)
* [Support](#support)
* [Copyright & License](#copyright-&-license)
* [Copyright and License](#copyright-and-license)
<!--- END -->
## Screenshots
Here are some early screenshots of the application:
Here are some screenshots of the application:
<!--
Commands run before taking the screenshots:
@@ -47,9 +49,9 @@ And to exit demo mode:
adb shell am broadcast -a com.android.systemui.demo -e command exit
-->
|<img src=./docs/images-lfs/screen_1_light.png width=280 />|<img src=./docs/images-lfs/screen_2_light.png width=280 />|<img src=./docs/images-lfs/screen_3_light.png width=280 />|<img src=./docs/images-lfs/screen_4_light.png width=280 />|
|<img src="./docs/images-lfs/screen_1_light.png" width="280" />|<img src="./docs/images-lfs/screen_2_light.png" width="280" />|<img src="./docs/images-lfs/screen_3_light.png" width="280" />|<img src="./docs/images-lfs/screen_4_light.png" width="280" />|
|-|-|-|-|
|<img src=./docs/images-lfs/screen_1_dark.png width=280 />|<img src=./docs/images-lfs/screen_2_dark.png width=280 />|<img src=./docs/images-lfs/screen_3_dark.png width=280 />|<img src=./docs/images-lfs/screen_4_dark.png width=280 />|
|<img src="./docs/images-lfs/screen_1_dark.png" width="280" />|<img src="./docs/images-lfs/screen_2_dark.png" width="280" />|<img src="./docs/images-lfs/screen_3_dark.png" width="280" />|<img src="./docs/images-lfs/screen_4_dark.png" width="280" />|
## Translations
@@ -90,7 +92,7 @@ When you are experiencing an issue on Element X Android, please first search in
and then in [#element-x-android:matrix.org](https://matrix.to/#/#element-x-android:matrix.org).
If after your research you still have a question, ask at [#element-x-android:matrix.org](https://matrix.to/#/#element-x-android:matrix.org). Otherwise feel free to create a GitHub issue if you encounter a bug or a crash, by explaining clearly in detail what happened. You can also perform bug reporting from the application settings. This is especially recommended when you encounter a crash.
## Copyright & License
## Copyright and License
Copyright © New Vector Ltd

View File

@@ -223,7 +223,6 @@ dependencies {
allLibrariesImpl()
allServicesImpl()
allFeaturesImpl(rootDir, logger)
implementation(projects.features.call)
implementation(projects.features.migration.api)
implementation(projects.anvilannotations)
implementation(projects.appnav)

View File

@@ -1,20 +0,0 @@
<?xml version="1.0" encoding="utf-8"?>
<!--
~ Copyright (c) 2023 New Vector Ltd
~
~ Licensed under the Apache License, Version 2.0 (the "License");
~ you may not use this file except in compliance with the License.
~ You may obtain a copy of the License at
~
~ http://www.apache.org/licenses/LICENSE-2.0
~
~ Unless required by applicable law or agreed to in writing, software
~ distributed under the License is distributed on an "AS IS" BASIS,
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
~ See the License for the specific language governing permissions and
~ limitations under the License.
-->
<resources xmlns:tools="http://schemas.android.com/tools">
<!-- The https://github.com/LikeTheSalad/android-stem requires a non empty strings.xml -->
<string name="ignored_placeholder" translatable="false" tools:ignore="UnusedResources">ignored</string>
</resources>

View File

@@ -40,9 +40,4 @@ object ApplicationConfig {
* For Element, the value is "Element". We use the same name for desktop and mobile for now.
*/
const val DESKTOP_APPLICATION_NAME: String = "Element"
/**
* The maximum size of the upload request. Default value is just below CloudFlare's max request size.
*/
const val MAX_LOG_UPLOAD_SIZE = 50 * 1024 * 1024L
}

View File

@@ -0,0 +1,36 @@
/*
* Copyright (c) 2024 New Vector Ltd
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.element.android.appconfig
object RageshakeConfig {
/**
* The URL to submit bug reports to.
*/
const val BUG_REPORT_URL = "https://riot.im/bugreports/submit"
/**
* As per https://github.com/matrix-org/rageshake:
* Identifier for the application (eg 'riot-web').
* Should correspond to a mapping configured in the configuration file for github issue reporting to work.
*/
const val BUG_REPORT_APP_NAME = "element-x-android"
/**
* The maximum size of the upload request. Default value is just below CloudFlare's max request size.
*/
const val MAX_LOG_UPLOAD_SIZE = 50 * 1024 * 1024L
}

View File

@@ -50,7 +50,6 @@ dependencies {
implementation(projects.libraries.permissions.api)
implementation(projects.libraries.permissions.noop)
implementation(projects.services.toolbox.api)
implementation(projects.services.toolbox.test)
testImplementation(libs.test.junit)
testImplementation(libs.coroutines.test)
@@ -63,6 +62,7 @@ dependencies {
testImplementation(projects.libraries.permissions.test)
testImplementation(projects.libraries.preferences.test)
testImplementation(projects.features.lockscreen.test)
testImplementation(projects.services.toolbox.test)
testImplementation(projects.tests.testutils)
ksp(libs.showkase.processor)

View File

@@ -58,6 +58,6 @@ dependencies {
testImplementation(projects.libraries.cryptography.test)
testImplementation(projects.libraries.cryptography.impl)
testImplementation(projects.libraries.featureflag.test)
implementation(projects.libraries.sessionStorage.test)
implementation(projects.services.appnavstate.test)
testImplementation(projects.libraries.sessionStorage.test)
testImplementation(projects.services.appnavstate.test)
}

View File

@@ -1,20 +0,0 @@
<?xml version="1.0" encoding="utf-8"?>
<!--
~ Copyright (c) 2023 New Vector Ltd
~
~ Licensed under the Apache License, Version 2.0 (the "License");
~ you may not use this file except in compliance with the License.
~ You may obtain a copy of the License at
~
~ http://www.apache.org/licenses/LICENSE-2.0
~
~ Unless required by applicable law or agreed to in writing, software
~ distributed under the License is distributed on an "AS IS" BASIS,
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
~ See the License for the specific language governing permissions and
~ limitations under the License.
-->
<manifest>
</manifest>

View File

@@ -1,6 +0,0 @@
<?xml version="1.0" encoding="utf-8"?>
<resources xmlns:xliff="urn:oasis:names:tc:xliff:document:1.2">
<!-- TODO Delete those 2 strings (replaced by action_sign_out) -->
<!--string name="screen_signout_confirmation_dialog_submit">"Sign out"</string>
<string name="screen_signout_preference_item">"Sign out"</string-->
</resources>

View File

@@ -21,12 +21,11 @@ import android.os.Build
import androidx.core.net.toFile
import androidx.core.net.toUri
import com.squareup.anvil.annotations.ContributesBinding
import io.element.android.appconfig.ApplicationConfig
import io.element.android.appconfig.RageshakeConfig
import io.element.android.features.rageshake.api.crash.CrashDataStore
import io.element.android.features.rageshake.api.reporter.BugReporter
import io.element.android.features.rageshake.api.reporter.BugReporterListener
import io.element.android.features.rageshake.api.screenshot.ScreenshotHolder
import io.element.android.features.rageshake.impl.R
import io.element.android.libraries.androidutils.file.compressFile
import io.element.android.libraries.androidutils.file.safeDelete
import io.element.android.libraries.core.coroutine.CoroutineDispatchers
@@ -139,7 +138,7 @@ class DefaultBugReporter @Inject constructor(
// build the multi part request
val builder = BugReporterMultipartBody.Builder()
.addFormDataPart("text", bugDescription)
.addFormDataPart("app", context.getString(R.string.bug_report_app_name))
.addFormDataPart("app", RageshakeConfig.BUG_REPORT_APP_NAME)
.addFormDataPart("user_agent", userAgentProvider.provide())
.addFormDataPart("user_id", userId?.toString() ?: "undefined")
.addFormDataPart("can_contact", canContact.toString())
@@ -178,7 +177,7 @@ class DefaultBugReporter @Inject constructor(
val requestBody = file.asRequestBody(MimeTypes.OctetStream.toMediaTypeOrNull())
totalUploadedSize += requestBody.contentLength()
// If we are about to upload more than the max request size, stop here
if (totalUploadedSize > ApplicationConfig.MAX_LOG_UPLOAD_SIZE) {
if (totalUploadedSize > RageshakeConfig.MAX_LOG_UPLOAD_SIZE) {
Timber.e("Could not upload file ${file.name} because it would exceed the max request size")
break
}
@@ -375,7 +374,7 @@ class DefaultBugReporter @Inject constructor(
val separator = System.lineSeparator()
logcatProcess.inputStream
.reader()
.buffered(ApplicationConfig.MAX_LOG_UPLOAD_SIZE.toInt())
.buffered(RageshakeConfig.MAX_LOG_UPLOAD_SIZE.toInt())
.forEachLine { line ->
streamWriter.append(line)
streamWriter.append(separator)

View File

@@ -17,18 +17,15 @@
package io.element.android.features.rageshake.impl.reporter
import com.squareup.anvil.annotations.ContributesBinding
import io.element.android.features.rageshake.impl.R
import io.element.android.appconfig.RageshakeConfig
import io.element.android.libraries.di.AppScope
import io.element.android.services.toolbox.api.strings.StringProvider
import okhttp3.HttpUrl
import okhttp3.HttpUrl.Companion.toHttpUrl
import javax.inject.Inject
@ContributesBinding(AppScope::class)
class DefaultBugReporterUrlProvider @Inject constructor(
private val stringProvider: StringProvider
) : BugReporterUrlProvider {
class DefaultBugReporterUrlProvider @Inject constructor() : BugReporterUrlProvider {
override fun provide(): HttpUrl {
return stringProvider.getString(R.string.bug_report_url).toHttpUrl()
return RageshakeConfig.BUG_REPORT_URL.toHttpUrl()
}
}

View File

@@ -1,29 +0,0 @@
<?xml version="1.0" encoding="utf-8"?><!--
~ Copyright (c) 2022 New Vector Ltd
~
~ Licensed under the Apache License, Version 2.0 (the "License");
~ you may not use this file except in compliance with the License.
~ You may obtain a copy of the License at
~
~ http://www.apache.org/licenses/LICENSE-2.0
~
~ Unless required by applicable law or agreed to in writing, software
~ distributed under the License is distributed on an "AS IS" BASIS,
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
~ See the License for the specific language governing permissions and
~ limitations under the License.
-->
<resources>
<!-- Rageshake configuration -->
<string name="bug_report_url" translatable="false">https://riot.im/bugreports/submit</string>
<!--
As per https://github.com/matrix-org/rageshake
bug_report_app_name: Identifier for the application (eg 'riot-web').
Should correspond to a mapping configured in the configuration file for github issue reporting to work.
-->
<string name="bug_report_app_name" translatable="false">element-x-android</string>
</resources>

View File

@@ -17,15 +17,15 @@
package io.element.android.features.rageshake.impl.reporter
import com.google.common.truth.Truth.assertThat
import io.element.android.services.toolbox.test.strings.FakeStringProvider
import io.element.android.appconfig.RageshakeConfig
import okhttp3.HttpUrl.Companion.toHttpUrl
import org.junit.Test
class DefaultBugReporterUrlProviderTest {
@Test
fun `test DefaultBugReporterUrlProvider`() {
val sut = DefaultBugReporterUrlProvider(FakeStringProvider("https://example.org"))
val sut = DefaultBugReporterUrlProvider()
val result = sut.provide()
assertThat(result).isEqualTo("https://example.org".toHttpUrl())
assertThat(result).isEqualTo(RageshakeConfig.BUG_REPORT_URL.toHttpUrl())
}
}

View File

@@ -1,16 +0,0 @@
<?xml version="1.0" encoding="utf-8"?><!--
~ Copyright (c) 2023 New Vector Ltd
~
~ Licensed under the Apache License, Version 2.0 (the "License");
~ you may not use this file except in compliance with the License.
~ You may obtain a copy of the License at
~
~ http://www.apache.org/licenses/LICENSE-2.0
~
~ Unless required by applicable law or agreed to in writing, software
~ distributed under the License is distributed on an "AS IS" BASIS,
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
~ See the License for the specific language governing permissions and
~ limitations under the License.
-->
<manifest />

View File

@@ -1,16 +0,0 @@
<?xml version="1.0" encoding="utf-8"?><!--
~ Copyright (c) 2023 New Vector Ltd
~
~ Licensed under the Apache License, Version 2.0 (the "License");
~ you may not use this file except in compliance with the License.
~ You may obtain a copy of the License at
~
~ http://www.apache.org/licenses/LICENSE-2.0
~
~ Unless required by applicable law or agreed to in writing, software
~ distributed under the License is distributed on an "AS IS" BASIS,
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
~ See the License for the specific language governing permissions and
~ limitations under the License.
-->
<manifest />

View File

@@ -1,16 +0,0 @@
<?xml version="1.0" encoding="utf-8"?><!--
~ Copyright (c) 2023 New Vector Ltd
~
~ Licensed under the Apache License, Version 2.0 (the "License");
~ you may not use this file except in compliance with the License.
~ You may obtain a copy of the License at
~
~ http://www.apache.org/licenses/LICENSE-2.0
~
~ Unless required by applicable law or agreed to in writing, software
~ distributed under the License is distributed on an "AS IS" BASIS,
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
~ See the License for the specific language governing permissions and
~ limitations under the License.
-->
<manifest />

View File

@@ -1,20 +0,0 @@
<?xml version="1.0" encoding="utf-8"?>
<!--
~ Copyright (c) 2022 New Vector Ltd
~
~ Licensed under the Apache License, Version 2.0 (the "License");
~ you may not use this file except in compliance with the License.
~ You may obtain a copy of the License at
~
~ http://www.apache.org/licenses/LICENSE-2.0
~
~ Unless required by applicable law or agreed to in writing, software
~ distributed under the License is distributed on an "AS IS" BASIS,
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
~ See the License for the specific language governing permissions and
~ limitations under the License.
-->
<manifest>
</manifest>

View File

@@ -1,18 +0,0 @@
<?xml version="1.0" encoding="utf-8"?>
<!--
~ Copyright (c) 2022 New Vector Ltd
~
~ Licensed under the Apache License, Version 2.0 (the "License");
~ you may not use this file except in compliance with the License.
~ You may obtain a copy of the License at
~
~ http://www.apache.org/licenses/LICENSE-2.0
~
~ Unless required by applicable law or agreed to in writing, software
~ distributed under the License is distributed on an "AS IS" BASIS,
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
~ See the License for the specific language governing permissions and
~ limitations under the License.
-->
<manifest/>

View File

@@ -16,7 +16,6 @@
<resources>
<!-- TODO EAx -->
<color name="notification_accent_color">#368BD6</color>
</resources>

View File

@@ -1,17 +0,0 @@
<?xml version="1.0" encoding="utf-8"?><!--
~ Copyright (c) 2023 New Vector Ltd
~
~ Licensed under the Apache License, Version 2.0 (the "License");
~ you may not use this file except in compliance with the License.
~ You may obtain a copy of the License at
~
~ http://www.apache.org/licenses/LICENSE-2.0
~
~ Unless required by applicable law or agreed to in writing, software
~ distributed under the License is distributed on an "AS IS" BASIS,
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
~ See the License for the specific language governing permissions and
~ limitations under the License.
-->
<manifest />

View File

@@ -67,7 +67,6 @@ dependencies {
// `testOptions { unitTests.isIncludeAndroidResources = true }` in the app build.gradle.kts file
// implementation(projects.app)
implementation(projects.appnav)
implementation(projects.features.call)
allLibrariesImpl()
allServicesImpl()
allFeaturesImpl(rootDir, logger)

View File

@@ -0,0 +1,82 @@
#!/usr/bin/env python3
import os
import subprocess
def getProjectDependencies():
print("=> Computing dependencies...")
command = subprocess.run(
["./gradlew :app:dependencies"],
shell=True,
capture_output=True,
text=True,
)
data = command.stdout
# Remove the trailing info like "(*)"
result = list(map(lambda x: x.split(" (")[0], data.split("\n")))
# Filter out comment line
result = list(filter(lambda x: "--- project" in x, result))
return result
def checkThatModulesExist(dependencies):
print("=> Checking that all modules exist...")
error = 0
modules = set()
for line in dependencies:
if line:
line = line.split(" ")
for elem in line:
if ":" in elem:
modules.add(elem)
for module in modules:
path = "." + module.replace(":", "/") + "/build.gradle.kts"
if not os.path.exists(path):
error += 1
print("Error: there is at least one dependency to '" + module + "' but the module does not exist.")
print(" Please remove occurrence(s) of 'implementation(projects" + module.replace(":", ".") + ")'.")
return error
def checkThatThereIsNoTestDependency(dependencies):
print("=> Checking that there are no test dependencies...")
errors = set()
currentProject = ""
for line in dependencies:
if line.startswith("+--- project "):
currentProject = line.split(" ")[2]
else:
if ":test" in currentProject:
continue
else:
subProject = line.split(" ")[-1]
if subProject.endswith(":test") or ":tests:" in subProject:
error = "Error: '" + currentProject + "' depends on the test project '" + subProject + "'\n"
error += " Please replace occurrence(s) of 'implementation(projects" + subProject.replace(":", ".") + ")'"
error += " with 'testImplementation(projects" + subProject.replace(":", ".") + ")'."
errors.add(error)
for error in errors:
print(error)
return len(errors)
def main():
dependencies = getProjectDependencies()
# for dep in dependencies:
# print(dep)
errors = 0
errors += checkThatModulesExist(dependencies)
errors += checkThatThereIsNoTestDependency(dependencies)
print()
if (errors == 0):
print("All checks passed successfully.")
elif (errors == 1):
print("Please fix the error above.")
else:
print("Please fix the " + str(errors) + " errors above.")
exit(errors)
if __name__ == "__main__":
main()