From 57ec240612ff1deb8343fcbb820852b6d3967299 Mon Sep 17 00:00:00 2001 From: Praneeth Date: Fri, 17 Mar 2023 17:22:15 +0530 Subject: [PATCH] Fix #4622: Android Wiki migration (#4893) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Explanation - Fixes #4622 - The PR does the following: This PR moves the android wiki into the oppia-andoird codebase allowing contributors to make PRs to the wiki along with changes to the codebase itself. This PR also introduces a GitHub Action (Runs when a PR is merged into develop and when a user makes change to the wiki via the web interface) that copies the content of the wiki folder along with the git history of the wiki folder and deploys it to the oppia-android wiki repository. The steps followed to achieve this wiki migration are present here: [Oppia Android Wiki Migration V2 (1).pdf](https://github.com/oppia/oppia-android/files/10921370/Oppia.Android.Wiki.Migration.V2.1.pdf) ### Note: Please ensure GitHub actions have write permissions. Please follow these instructions to enable it: [GrantingReadandWritePermissionsonGitHub_PDF.pdf](https://github.com/oppia/oppia-android/files/10919834/GrantingReadandWritePermissionsonGitHub_PDF.pdf) ## Essential Checklist - [x] The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".) - [x] Any changes to [scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets) files have their rationale included in the PR explanation. - [x] The PR follows the [style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide). - [x] The PR does not contain any unnecessary code changes from Android Studio ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)). - [x] The PR is made from a branch that's **not** called "develop" and is up-to-date with "develop". - [x] The PR is **assigned** to the appropriate reviewers ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)). ## For UI-specific PRs only If your PR includes UI-related changes, then: - Add screenshots for portrait/landscape for both a tablet & phone of the before & after UI changes - For the screenshots above, include both English and pseudo-localized (RTL) screenshots (see [RTL guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines)) - Add a video showing the full UX flow with a screen reader enabled (see [accessibility guide](https://github.com/oppia/oppia-android/wiki/Accessibility-(A11y)-Guide)) - Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing --------- Co-authored-by: Rajat Talesra Co-authored-by: Ben Henning Co-authored-by: Akshay Nandwana Co-authored-by: Srikanth K B Co-authored-by: Jan Co-authored-by: Abhay Garg <54636525+prayutsu@users.noreply.github.com> Co-authored-by: Arjun Gupta Co-authored-by: Sparsh Agrawal <55937724+Sparsh1212@users.noreply.github.com> Co-authored-by: Farees Hussain Co-authored-by: Sarthak Agarwal Co-authored-by: Sean Lip Co-authored-by: Apoorv Srivastava <53645584+MaskedCarrot@users.noreply.github.com> Co-authored-by: Vinita Murthi Co-authored-by: Ankita Saxena Co-authored-by: Vojtěch Jelínek Co-authored-by: Ben Henning --- .github/CODEOWNERS | 3 + .github/PULL_REQUEST_TEMPLATE.md | 2 +- .github/workflows/wiki.yml | 27 ++ scripts/assets/todo_open_exemptions.textproto | 21 ++ wiki/Accessibility-A11y-Guide.md | 144 ++++++++ wiki/Background-Processing.md | 46 +++ wiki/Bazel-Setup-Instructions-for-Windows.md | 167 +++++++++ wiki/Buf-Guide.md | 76 ++++ wiki/Coding-style-guide.md | 79 ++++ wiki/Contributing-to-Oppia-Android.md | 103 ++++++ wiki/Creating-a-screenshot.md | 44 +++ wiki/Dagger.md | 132 +++++++ wiki/Dark-Mode.md | 98 +++++ wiki/DataProvider-&-LiveData.md | 47 +++ wiki/Dependency-Injection.md | 52 +++ wiki/Developing-skills.md | 20 + wiki/End-to-End-Testing-Guide.md | 111 ++++++ wiki/Firebase-Console-Guide.md | 114 ++++++ wiki/Fork-and-Clone-Oppia-Android.md | 41 +++ wiki/Frequent-Errors-and-Solutions.md | 49 +++ wiki/Get-Help.md | 158 ++++++++ ...-Bazel-Migration-Best-Practices-and-FAQ.md | 61 ++++ wiki/Guidance-on-submitting-a-PR.md | 212 +++++++++++ wiki/Home.md | 13 + wiki/Installing-Oppia-Android.md | 68 ++++ wiki/Instructions-for-making-a-code-change.md | 79 ++++ wiki/Internationalization.md | 39 ++ wiki/Interpreting-CI-Results.md | 16 + wiki/Kotlin-Coroutines.md | 18 + wiki/Ktlint-Guide.md | 53 +++ wiki/Oppia-Android-Testing.md | 343 ++++++++++++++++++ wiki/Oppia-Bazel-Setup-Instructions.md | 120 ++++++ wiki/Our-Mission.md | 25 ++ ...Oppia-Android-codebase-and-architecture.md | 186 ++++++++++ ...ntCacheStore-&-In-Memory-Blocking-Cache.md | 16 + wiki/Platform-Parameters-&-Feature-Flags.md | 152 ++++++++ wiki/README.md | 1 + wiki/RTL-Guidelines.md | 48 +++ wiki/Revert-&-regression-policy.md | 35 ++ wiki/SLoP-2020.md | 73 ++++ wiki/Static-Analysis-Checks.md | 215 +++++++++++ wiki/Teams-at-Oppia-Android.md | 44 +++ wiki/Troubleshooting-Installation.md | 66 ++++ wiki/Updating-Maven-Dependencies.md | 140 +++++++ wiki/Wiki.md | 82 +++++ wiki/Work-Manager.md | 164 +++++++++ wiki/Working-on-UI.md | 73 ++++ wiki/Writing-design-docs.md | 49 +++ wiki/_Footer.md | 1 + wiki/_Sidebar.md | 55 +++ 50 files changed, 3980 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/wiki.yml create mode 100644 wiki/Accessibility-A11y-Guide.md create mode 100644 wiki/Background-Processing.md create mode 100644 wiki/Bazel-Setup-Instructions-for-Windows.md create mode 100644 wiki/Buf-Guide.md create mode 100644 wiki/Coding-style-guide.md create mode 100644 wiki/Contributing-to-Oppia-Android.md create mode 100644 wiki/Creating-a-screenshot.md create mode 100644 wiki/Dagger.md create mode 100644 wiki/Dark-Mode.md create mode 100644 wiki/DataProvider-&-LiveData.md create mode 100644 wiki/Dependency-Injection.md create mode 100644 wiki/Developing-skills.md create mode 100644 wiki/End-to-End-Testing-Guide.md create mode 100644 wiki/Firebase-Console-Guide.md create mode 100644 wiki/Fork-and-Clone-Oppia-Android.md create mode 100644 wiki/Frequent-Errors-and-Solutions.md create mode 100644 wiki/Get-Help.md create mode 100644 wiki/Gradle--Bazel-Migration-Best-Practices-and-FAQ.md create mode 100644 wiki/Guidance-on-submitting-a-PR.md create mode 100644 wiki/Home.md create mode 100644 wiki/Installing-Oppia-Android.md create mode 100644 wiki/Instructions-for-making-a-code-change.md create mode 100644 wiki/Internationalization.md create mode 100644 wiki/Interpreting-CI-Results.md create mode 100644 wiki/Kotlin-Coroutines.md create mode 100644 wiki/Ktlint-Guide.md create mode 100644 wiki/Oppia-Android-Testing.md create mode 100644 wiki/Oppia-Bazel-Setup-Instructions.md create mode 100644 wiki/Our-Mission.md create mode 100644 wiki/Overview-of-the-Oppia-Android-codebase-and-architecture.md create mode 100644 wiki/PersistentCacheStore-&-In-Memory-Blocking-Cache.md create mode 100644 wiki/Platform-Parameters-&-Feature-Flags.md create mode 100644 wiki/README.md create mode 100644 wiki/RTL-Guidelines.md create mode 100644 wiki/Revert-&-regression-policy.md create mode 100644 wiki/SLoP-2020.md create mode 100644 wiki/Static-Analysis-Checks.md create mode 100644 wiki/Teams-at-Oppia-Android.md create mode 100644 wiki/Troubleshooting-Installation.md create mode 100644 wiki/Updating-Maven-Dependencies.md create mode 100644 wiki/Wiki.md create mode 100644 wiki/Work-Manager.md create mode 100644 wiki/Working-on-UI.md create mode 100644 wiki/Writing-design-docs.md create mode 100644 wiki/_Footer.md create mode 100644 wiki/_Sidebar.md diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 3943cd16563..53455bd6ccd 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -48,6 +48,9 @@ gradlew.bat @BenHenning /.github/workflows/ @BenHenning /.github/stale.yml @BenHenning +# Wiki +/wiki/ @BenHenning + # Devbots configurations. /.devbots/ @BenHenning diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index a7a3232776b..53f6d452828 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -20,5 +20,5 @@ If your PR includes UI-related changes, then: - Add screenshots for portrait/landscape for both a tablet & phone of the before & after UI changes - For the screenshots above, include both English and pseudo-localized (RTL) screenshots (see [RTL guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines)) -- Add a video showing the full UX flow with a screen reader enabled (see [accessibility guide](https://github.com/oppia/oppia-android/wiki/Accessibility-(A11y)-Guide)) +- Add a video showing the full UX flow with a screen reader enabled (see [accessibility guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide)) - Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing diff --git a/.github/workflows/wiki.yml b/.github/workflows/wiki.yml new file mode 100644 index 00000000000..f37031f61e9 --- /dev/null +++ b/.github/workflows/wiki.yml @@ -0,0 +1,27 @@ +name: Deploy to Wiki +on: + push: + branches: + - develop + paths: + - 'wiki/**' + # Triggers this workflow when the wiki is changed + # (see https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#gollum). + gollum: + +jobs: + wiki-deploy: + runs-on: ${{ matrix.os }} + strategy: + matrix: + os: [ubuntu-20.04] + steps: + - uses: actions/checkout@v3 + with: + fetch-depth: 0 + - name: Add remote + run: | + git filter-branch --subdirectory-filter wiki/ -- --all + git remote set-url origin https://github.com/oppia/oppia-android.wiki.git + git checkout -b master + git push origin master --force diff --git a/scripts/assets/todo_open_exemptions.textproto b/scripts/assets/todo_open_exemptions.textproto index d6d2945190e..34352a5fca0 100644 --- a/scripts/assets/todo_open_exemptions.textproto +++ b/scripts/assets/todo_open_exemptions.textproto @@ -290,3 +290,24 @@ todo_open_exemption { exempted_file_path: "scripts/src/java/org/oppia/android/scripts/todo/model/Todo.kt" line_number: 10 } + +todo_open_exemption { + exempted_file_path: "wiki/Coding-style-guide.md" + line_number: 33 +} + +todo_open_exemption { + exempted_file_path: "wiki/Static-Analysis-Checks.md" + line_number: 155 + line_number: 163 + line_number: 168 + line_number: 172 + line_number: 176 + line_number: 178 + line_number: 193 + line_number: 203 + line_number: 206 + line_number: 209 + line_number: 212 + line_number: 215 +} diff --git a/wiki/Accessibility-A11y-Guide.md b/wiki/Accessibility-A11y-Guide.md new file mode 100644 index 00000000000..603d1e28f45 --- /dev/null +++ b/wiki/Accessibility-A11y-Guide.md @@ -0,0 +1,144 @@ +## Overview +Accessibility is an important part of Oppia to ensure that the app is accessible by everyone. Some common conditions that affect a person's use of an Android device are: +* blindness / low vision +* deafness / impaired hearing +* cognitive disabilities +* confined motor skills +* color blindness, etc. + +Making sure that the Oppia app is accessible by all resonates with overall Oppia's mission: **to help anyone learn anything they want in an effective and enjoyable way.** + +Note: In short we can write Accessibility as **A11Y**. + +## How to test the app for a11y users? +There are various manual and automated tests to check if app is accessible by all or not. All of the below mentioned tests are required to make sure that app is accessible in most cases. + +**[Accessibility Scanner](https://support.google.com/accessibility/android/answer/6376570?hl=en)** : Using Accessibility Scanner we can take screenshots of each and every screen in the Oppia-app manually and the Accessibility Scanner app will give the output for the individual screenshot mentioning all the errors. + +**Screen Reader**: Screen readers like **Talkback** can be used to test the app manually. Talkback app is used by blind people to navigate to different items in the screen and get audio based output. This app will not give any error like Accessibility Scanner. + +**[AccessibilityChecks](https://developer.android.com/guide/topics/ui/accessibility/testing#automated)**: Developers can activate the `AccessibilityChecks` in all `Espresso` test cases which will give errors related to accessibility. + + +## Setting up Accessibility Scanner and Talkback + +### Setup Play Store in mobile emulator +1. Create a new emulator device which contains **Google Play Store** in it. Example: Nexus 5, Nexus 5A, Pixel, Pixel 2, Pixel 3, Pixel 3a, Pixel 4. +2. Open "Play Store" app and sign-in. + +### Setup Play Store in tablet emulator +By default tablet emulators do not contain **Play Store** app and therefore you will need to make some changes to make it available. +1. Follow the steps mentioned [here](https://stackoverflow.com/a/62680014). +2. Once the above steps are done, start the emulator. +3. Open the "Play Store" app and sign-in. + +### Using a11y scanner in android +[Accessibility Scanner](https://support.google.com/accessibility/android/answer/6376570?hl=en) scans your screen and provides suggestions to improve the accessibility of your app, based on: +* Content labels +* Touch target size +* Clickable items +* Text and image contrast + +#### How to Use? +1. Open **Google Play Store** +2. Download/Install **Accessibility Scanner** app +3. After installation, open Settings +4. Search **Accessibility Scanner**, click on it. +5. Turn on **Use Accessibility Scanner** -> **Allow** +6. You will notice a blue colored floating button with tick/check icon. +7. Open **Oppia** app. +8. Now on any screen inside app click on the floating button and either record or take snapshot. + +**Result**: You will notice that the scanner analyses the screen and give errors if there are any or else it will show that no errors were found. + +### Using Talkback in android +TalkBack is the Google **screen reader** included on Android devices. TalkBack gives you spoken feedback so that you can use your device without looking at the screen. + +#### How to use? +1. Open **Google Play Store** +2. Download/Install **Android Accessibility Suite** app +3. After installation, open Settings +4. Search **Talkback**, click on it. +5. Read all the instructions written on the screen as using Talkback requires specific steps. +6. Turn on **Use Service** -> **Allow** + + +### Useful Resources +* [Android A11Y Overview](https://support.google.com/accessibility/android/answer/6006564) +* [Using A11Y Menu](https://support.google.com/accessibility/android/answer/9078941) +* [Getting started with Talkback](https://support.google.com/accessibility/android/answer/6283677) +* [Display speech output as Text: Talkback](https://developer.android.com/guide/topics/ui/accessibility/testing#optional_talkback_developer_settings) + + +## Using AccessibilityTestRule in Espresso Tests +[AccessibilityTestRule](https://github.com/oppia/oppia-android/blob/develop/testing/src/main/java/org/oppia/android/testing/AccessibilityTestRule.kt) is a JUnit rule to enable `AccessibilityChecks` in all Espresso Tests. This rule covers all errors shown by Accessibility Scanner and more but only for all those UI elements which are getting used in the test case. + +(**Note: If this file is not available then it has been merged with OppiaTestRule as per #3351**) + +#### How to use? +Simply use the `AccessibilityTestRule` in Espresso Test file like this: +``` +@get: Rule +val accessiblityTestRule = AccessibilityTestRule() +``` + +This will enable the `AccessibilityChecks` on all Espresso tests inside this file. + +To disable `AccessibilityChecks` on any individual test case use [DisableAccessibilityChecks](https://github.com/oppia/oppia-android/blob/develop/testing/src/main/java/org/oppia/android/testing/DisableAccessibilityChecks.kt) as annotation with the test. + +In case of test failure there are two options to fix it: +* Solve the test case by updating the UI as per the error details. +* If by solving the error the user experience will become worse than we should actually suppress that error instead of changing the UI. This can be done in [AccessibilityTestRule](https://github.com/oppia/oppia-android/blob/fe553d32e0161f6efa6e465109306b909dbcc476/testing/src/main/java/org/oppia/android/testing/AccessibilityTestRule.kt#L34) + +## Auditing the app +The app should be audited by covering different use cases across 23 different manual tests mentioned [here](https://docs.google.com/spreadsheets/d/1lFQo2XE0dSGZcMvr7paxdL3zXB3FVcRnZOqD70DT3a4/edit?usp=sharing). +This sheet has been divided based on `primary` and `secondary` use cases and `basic` and `advanced` test cases. +* Level 1 = `Primary + Basic`, which means that all primary use cases passes all `Basic` tests. +* Level 2 = `Secondary + Basic`, which means that all secondary use cases passes all `Basic` tests. +* Level 3 = `Primary + Advanced`, which means that all primary use cases passes all `Advanced` tests. +* Level 4 = `Secondary + Advanced`, which means that all secondary use cases passes all `Advanced` tests. + +This entire sheet should be filled with each release as a part of audit process. + +## General Tips to make app Accessible +* All Clickable items should have a minimum size of `48x48dp`. +* Buttons should use `android:enabled` instead of `android:clickable` to disable/enable it. +* All views should have a `foreground:background` contrast ratio of `4.5:1` and above. +* Texts should have a minimum `12sp` text size. +* Images/icons should have a meaningful content description. + +## Exceptional Cases +* Generally we use `sp` only for text-size/font-size of text and at all attributes related to width/height we use `dp`. But we can use width in `sp` if we have text inside a fixed width container. This will increase the size of container whenever we increase the font size, so the scaled text get enough size to fit inside a container. If this case is applied anywhere in UI, please get confirmation from @BenHenning or @rt4914 . + +## Android 12 Warnings around TextViews in Fixed Layouts + +### Problem with fixed layout + +If we have scalable text inside a fixed width container then accessibility scanner is suggesting to improve text scaling, as if the text scales it won’t get enough space to expand inside a fixed width container. + + + +### Possible solution to fix it + +1. Change the fixed width to wrap_content and set minWidth. In this case, we can’t directly change the width into a wrap_content as all thumbnail images won’t have consistent width which leads to a problem as mentioned in issue [#4684](https://github.com/oppia/oppia-android/issues/4684). You can see the below screenshot for reference. + + + +2. As directly we can’t use wrap_content for width, another possible solution is to use scalable width i.e. instead of setting fixed width in dp we can set the width in sp. This will increase the container size every time we increase the font size, so the scaled text can fit inside the container. In this approach, accessibility scanner will still show suggestion to improve text scaling but from a UI perspective this approach works. For reference you can look at PR [#4695](https://github.com/oppia/oppia-android/pull/4695) + +3. Another approach would be to design such that we don’t have to fit a text inside a fixed width container as shown below in the reference image. + + + +We can set the full width to the cards i.e. match_parent (with appropriate margins) which will remove the issue of accessibility. We can also show the dots at bottom which represent the number + position of items. + + + +For sighted users +- The banners will be cyclic i.e. item-0, item1, item2, item0 and repeat. + +For talkback users +- The cyclic nature will stop. +- The screen reader will start from item-0, item-1, item-2, and next it will go out of list. + + diff --git a/wiki/Background-Processing.md b/wiki/Background-Processing.md new file mode 100644 index 00000000000..c7a4aecb665 --- /dev/null +++ b/wiki/Background-Processing.md @@ -0,0 +1,46 @@ +## Overview + +This page aims to provide some context around: +- What constitutes background processing/when something should be background processed, and why we care +- How background processing is done in the Android app +- Best practices to follow when managing background or expensive tasks +- Existing utilities to simplify using DataProviders +- How to safely pass data to the UI + +## Background + +Performing asynchronous tasks or background processing in a multi-threaded environment is hard: most developers are unaware of the nuances of cross-thread development (e.g. properly utilizing critical sections without introducing deadlocks, sharing mutable state across thread boundaries with correctly applied memory fences, critical sections, or atomics, properly using concurrent data structures, and more). This problem is exacerbated in Android since: +1. Android requires all user-facing operations to be run on the main thread (requiring communication to/from the main thread) +2. Android UI objects are very [lifecycle](https://developer.android.com/guide/components/activities/activity-lifecycle)-sensitive (meaning haphazard management of Android state can at best leak memory or at worst crash when communicating back from a background thread--a common crash in Android apps) +3. The Android UI thread is sensitive to even medium-length operations when on slow devices (which can lead to app [ANR](https://developer.android.com/topic/performance/vitals/anr)s) + +The team has a number of carefully considered solutions to ensure concurrency is easier to manage, safer, and performant. + +### Definition of background processing & when to use it + +All features in the codebase can be represented as a data pipeline. In some cases, data is created transiently and in other cases it needs to be loaded from somewhere (e.g. a file or network). Further, we sometimes need to do a lot of processing on this data before it can be presented to the UI (the app's [architecture](https://github.com/oppia/oppia-android/wiki/Overview-of-the-Oppia-Android-codebase-and-architecture#app-architecture) is specifically designed to encourage data processing logic to live outside the UI). + +To keep things simple, we consider everything the following to be worth executing on a background thread instead of the UI thread: +- Any logic operation (e.g. something requiring an if statement or loop) which is more complicated than just copying data +- Any file I/O operations (e.g. reading from a file) +- Any networking operations (e.g. calling into Retrofit) +- Complex state management (such as [ExplorationProgressController](https://github.com/oppia/oppia-android/blob/a85399c2b0a2b9cf214881ce8c70d9b487f1e0b8/domain/src/main/java/org/oppia/android/domain/exploration/ExplorationProgressController.kt#L34)) + +Similarly, the following operations must happen on a UI thread for lifecycle safety reasons: +- Any interactions with the UI (e.g. activities, fragments, or views) +- Any interactions with ViewModel state (which is designed to only be mutated on the main thread) +- Any interactions with other Android services which require main thread access + +### Why we care + +Oppia Android is aiming to provide an effective education experience to the most underprivileged communities in the world, and this particularly requires excellent performance on low-end devices. We have a thin performance margin to operate in, and we can't afford ANRs or poor performance. Further, reducing crashes is important to ensure an uninterrupted learning experience (especially for children who might not understand how to recover the app from a crash). + +That being said, the difficulty in writing correct & performant concurrent code is quite high. We want to make sure we achieve that with a lower barrier-to-entry so that team members don't have to manage especially complex code. + +## Background processing & concurrency in Oppia Android + +To ensure the team is meeting the goals of reducing concurrency complexity while not sacrificing performance or correctness, we require that all code utilize the patterns & best practices outlined in this section. + +- [Kotlin Coroutines](https://github.com/oppia/oppia-android/wiki/Kotlin-Coroutines) +- [DataProvider & LiveData](https://github.com/oppia/oppia-android/wiki/DataProvider-&-LiveData) +- [PersistentCacheStore & In Memory Blocking Cache](https://github.com/oppia/oppia-android/wiki/PersistentCacheStore-&-In-Memory-Blocking-Cache) \ No newline at end of file diff --git a/wiki/Bazel-Setup-Instructions-for-Windows.md b/wiki/Bazel-Setup-Instructions-for-Windows.md new file mode 100644 index 00000000000..4425a088cee --- /dev/null +++ b/wiki/Bazel-Setup-Instructions-for-Windows.md @@ -0,0 +1,167 @@ +## Overview & Disclaimer + +This page outlines one way to allow Bazel to be used in CLI form on Windows. Please note that **this support is currently experimental**. You may run into some problems--we suggest that you [file an issue](https://github.com/oppia/oppia-android/issues/new/choose) ior contact us at [gitter](https://gitter.im/oppia/oppia-android). + +Unlike Unix-based systems where Bazel runs natively without issue, the current solution on Windows is to install an Ubuntu-based subsystem. Windows currently only supports a terminal experience in this subsystem (though there is a prerelease version of the software with GUI support) which means Android Studio will not be supported. You will need to continue using the Windows version of Android Studio and only use the Linux subsystem for building & running Robolectric or JUnit-based tests. + +## Instructions + +**Main prerequisites**: +- Windows 10: These instructions are geared towards users of Windows 10 (older versions will not be compatible). If you're in using an older version of Windows, please follow up with a comment on [this issue](https://github.com/oppia/oppia-android/issues/3371). +- At least 4GB of free local disk storage space (for Linux & needed dependencies), but more will probably help when building the app + +Also, note that these instructions *replace* the standard Oppia Bazel [set-up instructions](https://github.com/oppia/oppia-android/wiki/Oppia-Bazel-Setup-Instructions) since different steps are required for Windows with an Ubuntu subsystem. + +At a high-level, the steps to make Bazel work on Windows are: +1. Install the Ubuntu subsystem +2. Install prerequisite debian packages +3. Install the Android SDK +4. Install Bazel +5. Set up the environment to be able to build Oppia Android +6. Verify that the build is working +7. Run Bazel commands as needed during development + +### 1. Install Ubuntu subsystem + +Please follow Microsoft's [setup instructions](https://docs.microsoft.com/en-us/windows/wsl/install-win10) to set up the Linux subsystem on Windows 10. From there, you should install **Ubuntu** (the instructions below are based on the apt package manager on Ubuntu; other Linux distributions & package managers may work but they are untested by the team). + +Once installed, open the Ubuntu terminal from the start menu. + +### 2. Prerequisite debian packages + +From within the Ubuntu terminal, start by ensuring all packages are up-to-date: + +```sh +sudo apt update && sudo apt upgrade +``` + +After that, follow each of the subsections below as needed to install prerequisite dependencies: + +**Java** + +JDK 8 is required for the Android build tools, and we suggest installing OpenJDK: + +```sh +sudo apt install openjdk-8-jdk-headless +``` + +**Python 2** + +Unfortunately, some of the Bazel build actions in the Android pipeline require Python 2 to be installed: + +```sh +sudo apt install python +``` + +### 3. Installing the Android SDK + +We need to be able to run Linux-compatible Android utilities which requires installing a Linux version of the Android SDK. Since we can't install Android Studio in the subsystem, we need to do this via CLI commands. The steps below are extracted from [this article](https://proandroiddev.com/how-to-setup-android-sdk-without-android-studio-6d60d0f2812a). + +First, prepare the environment for the SDK by creating the default directory to hold the SDK (from within Ubuntu terminal): + +```sh +mkdir -p $HOME/Android/Sdk +``` + +Second, navigate to https://developer.android.com/studio#command-tools in a web browser (in Windows) and select to download the latest **Linux** command tools (even though you're using Windows, the Linux commandline tools are needed--the Windows version will not work with these instructions). Once downloaded, copy the zip file to the new SDK location (note that the ``/c/mnt/...`` path is based on ``C:\Users\\Downloads`` being the default download location--this may not be the case on your system) with your Windows username filled in for ````: + +```sh +cp /c/mnt/Users//Downloads/commandlinetools*.zip $HOME/Android/Sdk +``` + +After that, change to the directory, unzip the archive, and remove it: + +```sh +cd $HOME/Android/Sdk +unzip commandlinetools*.zip +rm commandlinetools*.zip +``` + +From there, the command line tools need to be moved in order to indicate to the tools themselves that they're relative to the Android SDK root directory: + +```sh +cd cmdline-tools/ +mkdir tools +mv -i * tools +cd .. +``` + +(The above may give a warning for the ``mv`` command since it will try moving ``tools`` into ``tools``--this can be ignored). + +At this point, we can define the ``ANDROID_HOME`` variable to point to the new SDK root, and also update the ``PATH`` to point to cmdnline-tools so that we can actually install the SDK. To do this, run the following commands to append new lines to ``~/.bashrc``: + +```sh +echo "export ANDROID_HOME=\$HOME/Android/Sdk" >> ~/.bashrc +echo "export PATH=\$ANDROID_HOME/cmdline-tools/tools/bin/:\$PATH" >> ~/.bashrc +source ~/.bashrc +``` + +(The last line reloads your Bash configuration file so that the variable adjustments above become live in your local terminal). + +The ``sdkmanager`` command can now be used to install the necessary packages. Run each of the following commands in succession (you may need to accept licenses for the SDK packages in the same way you would when using Android Studio): + +```sh +sdkmanager +sdkmanager --install "platform-tools" +sdkmanager --install "platforms;android-28" +sdkmanager --install "build-tools;29.0.2" +``` + +When the commands above are finished running, the Android SDK should now be installed in your subsystem & be accessible to Bazel. + +### 4. Installing Bazel + +Follow [these instructions](https://docs.bazel.build/versions/main/install-ubuntu.html#install-on-ubuntu) to install Bazel using ``apt`` rather than Bazelisk (Bazelisk might work, but it's untested with these instructions). Note that Oppia requires Bazel 4.0.0, so you'll likely need to run the following command: + +```sh +sudo apt install bazel-4.0.0 +``` + +### 5. Preparing build environment for Oppia Android + +The Oppia Android repository generally expects to live under an 'opensource' directory. While we recommend doing that in practice, we run into one complication when building the app on Windows: the repository itself lives under the native Windows filesystem & most of everything else needed to build lives under the Linux subsystem. To help simplify things, we prefer keeping just the repository on Windows and everything else on Linux, including the the Oppia Bazel toolchain. To prepare for this, we suggest making an 'opensource' directory in your Ubuntu subsystem: + +```sh +mkdir $HOME/opensource +``` + +From there, follow [these instructions](https://github.com/oppia/oppia-bazel-tools#readme) in order to prepare your environment to support Oppia Android builds. + +### 6. Verifying the build + +At this point, your system should be able to build Oppia Android. To verify, try building the APK (from your subsystem terminal -- note that this & all other Bazel commands must be run from the root of the ‘oppia-android’ directory otherwise they will fail): + +```sh +bazel build //:oppia +``` + +(Note that this command may take 10-20 minutes to complete depending on the performance of your machine). + +If everything is working, you should see output like the following: + +``` +Target //:oppia up-to-date: + bazel-bin/oppia_deploy.jar + bazel-bin/oppia_unsigned/apk + bazel-bin/oppia/apk +INFO: Elapsed time: ... +INFO: 1 process... +INFO: Build completed successfully, ... +``` + +If you see the above, you can proceed to use your subsystem for Bazel commands while developing Oppia. If you instead see an error, please [file an issue](https://github.com/oppia/oppia-android/issues/new/choose). + +Note also that the ``oppia.apk`` under the ``bazel-bin`` directory of your local copy of Oppia Android should be a fully functioning development version of the app that can be installed using ``adb`` (though we recommend using ADB from within a Windows command prompt or shell since the Ubuntu subsystem may not have correct support to access devices or emulators connected to the native Windows machine). + +### 7. Next steps + +At this point, commands listed on the other [Bazel setup instructions](https://github.com/oppia/oppia-android/wiki/Oppia-Bazel-Setup-Instructions#building-the-app) page should now work locally. Keep in mind that your development will continue on Windows via Android Studio and that the subsystem is only needed to execute Bazel commands (Gradle should continue to work through Windows). + +## Appendix + +### Limitations +Known limitations with using an Ubuntu subsystem on Windows: +- Android Studio must run on native Windows: this is a current limitation. However, https://github.com/microsoft/wslg is a WIP project that may provide an alternative option which allows full development to take place within the subsystem. +- The subsystem is very slow: unfortunately, this is just a limitation with how the subsystem works on Windows. Until we fix the actual build pipeline to work natively, this is likely going to be a limitation that we have to live with. Note that installing an Ubuntu VM or dual-booting Ubuntu may lead to less issues & better performance than using a subsystem, but this hasn't yet been investigated or documented yet (see [#3437](https://github.com/oppia/oppia-android/issues/3437) for the WIP issue). +- ADB is limited within the subsystem and thus must be used from within a Windows CLI like Command Prompt, Powershell, or Git Bash (if it's installed) in order to deploy the Bazel-built test or APK binary to an emulator or real device +- Emulators likely cannot be launched from the subsystem (headless might be possible, but this hasn't been tested) diff --git a/wiki/Buf-Guide.md b/wiki/Buf-Guide.md new file mode 100644 index 00000000000..e6931d53a92 --- /dev/null +++ b/wiki/Buf-Guide.md @@ -0,0 +1,76 @@ +# Installation +Once you had completed all the [installation steps](https://github.com/oppia/oppia-android/wiki#prerequisites), you will be having a `buf` file in your `opensource/oppia-android-tools` folder.
+**Note: Currently, Buf is not available for windows.** + +## Commands + +* Check the version of the buf. As of now on GitHub Actions, we are using `0.37.1`.
+`../oppia-android-tools/YOUR_BUF_FILE_NAME --version` +* Lint check
+`buf lint --config buf.yaml` + +## Configuration File + +We have a configuration file `buf.yaml` at the root of the project. Following is the list of things we are checking and the list of things we are excluding from our check. + +#### Checking: +* `DIRECTORY_SAME_PACKAGE` checks that all files in a given directory are in the same package. + * All proto files should be in the same package + +* `PACKAGE_SAME_DIRECTORY` checks that all files with a given package are in the same directory. + * All proto files with the same package name should be in the same directory + +* `PACKAGE_SAME_JAVA_MULTIPLE_FILES` checks that all files with a given package have the same value for the java_multiple_files option. + * All proto files should have the same value `option java_multiple_files = true;` + +* `PACKAGE_SAME_JAVA_PACKAGE` checks that all files with a given package have the same value for the java_package option. + * All proto files should have the same value `option java_package = "path.to.your.proto.directory";` + +* `ENUM_NO_ALLOW_ALIAS` checks that enums do not have the allow_alias option set. + * Should not allows multiple enum values to have the same number. + +* `FIELD_NO_DESCRIPTOR` checks that field names are not named capitalization of "descriptor" with any number of prefix or suffix underscores. + * No `descriptor` field name should be there, example - `descriptor`, `Descriptor`, `descRiptor`, `_descriptor`, `descriptor_`, `__descriptor__` + +* `IMPORT_NO_PUBLIC` checks that imports are not public. + * Import should not be declared as `public`, example - `import public "x.y.proto"` + +* `IMPORT_NO_WEAK` checks that imports are not weak. + * Import should not be declared as `weak`, example - `import weak "x.y.proto"` + +* `PACKAGE_DEFINED` checks that all files have a package defined. + * All proto files must have a package name specify in it, for example - `package model;` + +* `ENUM_PASCAL_CASE` checks that enums are PascalCase. + * Enum name should be in pascal case, example - `AudioLanguage` + +* `ENUM_VALUE_UPPER_SNAKE_CASE` checks that enum values are UPPER_SNAKE_CASE. + * Enum value should be in upper snake case + +* `FIELD_LOWER_SNAKE_CASE` checks that field names are lower_snake_case. + * Field name should be in lower snake case + +* `MESSAGE_PASCAL_CASE` checks that messages are PascalCase. + * Message should be in Pascal case, example - `ProfileAvatar` + +* `ONEOF_LOWER_SNAKE_CASE` checks that oneof names are lower_snake_case. + * Oneof should be a lower snake case + +* `PACKAGE_LOWER_SNAKE_CASE` checks that packages are lower_snake.case. + * Package should be lower snake case, example - `model` + +* `ENUM_ZERO_VALUE_SUFFIX` checks that enum zero values are suffixed with `_UNSPECIFIED` (the suffix is configurable). + * All the enum whose value is zero should be suffixed with `_UNSPECIFIED`, example - `AUDIO_LANGUAGE_UNSPECIFIED = 0;` + +* `FILE_LOWER_SNAKE_CASE` checks that filenames are lower_snake_case. + * All the proto file names are should be in lower snake case, for example - `topic.proto` + +#### Excluding: +* `PACKAGE_DIRECTORY_MATCH` checks that all files are in a directory that matches their package name. + * this verifies that all files that declare a given package foo.bar.baz.v1 is in the directory foo/bar/baz/v1 relative to the root + +* `ENUM_VALUE_PREFIX` checks that enum values are prefixed with `ENUM_NAME_UPPER_SNAKE_CASE`. + * `enum Foo { FOO_ONE = 0; }` + +* `PACKAGE_VERSION_SUFFIX` checks that the last component of all packages is a version of the form v\d+, v\d+test.*, v\d+(alpha|beta)\d+, or v\d+p\d+(alpha|beta)\d+, where numbers are >=1. + * `foo.v1` , `foo.bar.v1alpha1` , `foo.bar.v1beta1` , `foo.bar.v1test` diff --git a/wiki/Coding-style-guide.md b/wiki/Coding-style-guide.md new file mode 100644 index 00000000000..37773d4305e --- /dev/null +++ b/wiki/Coding-style-guide.md @@ -0,0 +1,79 @@ +Please follow the following style rules when writing code, in order to minimize unnecessary back-and-forth during code review. + +## General +- We follow the [Kotlin Android style guide](https://developer.android.com/kotlin/style-guide). +- Use 2 spaces for indentation and 4 spaces for continuation, per https://google.github.io/styleguide/javaguide.html#s4.2-block-indentation. (This should be configured at the project level for Kotlin. Ensure that you're using the project configuration for Kotlin in your IDE, so that you can reformat the code via the IDE if needed.) +- Never commit a PR which includes commented-out code. +- Ensure that your code looks consistent with the code surrounding it. +- Ensure that the indentation of your code is correct. +- In general, avoid leaving multiple consecutive blank lines. Collapse them into a single one. +- The last character in each file should always be a newline. (If it's not, you'll see a red symbol at the end of the file when you look at the "Files Changed" tab in GitHub.) +- Make sure to remove temporary code (e.g. log statements or toasts to help with local debugging) before pushing to GitHub. +- Do not check any build artifacts into GitHub. + +## Code Formatting +Reformat all edited files automatically in android studio using the following command. +- Windows: `Ctrl + Alt + L` +- Linux: `Ctrl + Shift + Alt + L` +- macOS: `Option + Command + L` +- Ubuntu users might face issue because `Ctrl + Alt + L` locks the screen by default nature. Refer to this [stackoverflow-link](https://stackoverflow.com/questions/16580171/code-formatting-shortcut-in-android-studio) on how to solve this. + +Alternatively, you can also use the in-built Android Studio option to reformat by selecting to `Code -> Reformat Code/File`. + +Screenshot 2021-11-09 at 6 25 26 PM + +NOTE: This does not guarantee 100% formatting of code as per guidelines but will be very helpful in indentation and extra spaces. + +## Comments +- Ensure that comments have correct grammar, spelling and capitalization. +- Vertically align the `*`s on the left side of a multi-line comment. +- Wrap Javadoc comments to the 120 character limit. +- Put new lines between Javadoc paragraphs. +- Do not leave any spaces between a Javadoc and the class/method/field that it's documenting. +- When writing TODOs, refer to an issue number on the GitHub issue tracker. E.g. `TODO(#1234): Do X, Y and Z.` + +## XML files +- Do not declare values directly in the XML file. Declare them in the corresponding resource file and use that variable. + +For example: +- For a dimension, declare it in dimens.xml file. +- For a string, declare it in strings.xml file. +- For a color, declare it in colors.xml file. + +In general, avoid using hard-coded strings. + +## Java/Kotlin files +- Separate adjacent functions or blocks of code by a single blank line. +- Do not use "magic numbers" in code. Declare constants instead (typically at the module level). +- Order imports alphabetically. Remove unused imports. You can make use of the in-built Android Studio option to optimise imports by selecting `Code -> Optimise Imports`. + +Screenshot 2021-11-09 at 6 25 05 PM + + +## Layout files +- Each layout file should be named according to how they are used, where all layouts fall in the following buckets: + - Activities: _screen\_name\_activity.xml_ (all end with ``_activity.xml``) + - Fragments: _subscreen\_name\_fragment.xml_ (all end with ``_fragment.xml``) + - Custom views: _custom\_widget\_name\_view.xml_ (all end with ``_view.xml``) + - RecyclerView items: _element\_name\_item.xml_ (all end with ``_item.xml``) + - Toolbars: _screen\_location\_toolbar.xml_ (all end with ``_toolbar.xml``) +- Any layouts not associated with the above that should be shared across multiple layouts should instead be associated with a custom view (including a corresponding Kotlin file). This means the ``include`` directive won't be included in any layouts. +- Since widget IDs within layout files are global scoped, they should be named based on their location, value, and widget type. + - The general format for this is: _\_\__ + - The following are some recognized widget types (this list is not comprehensive): + - ``TextView`` + - ``EditText`` + - ``RecyclerView`` + - ``Button`` + - ``View`` + - Custom views (the full name of the view should be spelled out for the widget type in identifiers) + - Layout elements should be named as follows: + - ``container``: if the element contains other elements within the same layout + - ``placeholder``: if the element will be replaced at runtime with new elements (such as a fragment layout) + - Here are some examples of valid IDs: + - ``recently_played_activity_recently_played_fragment_placeholder`` (a ``FrameLayout`` in ``recently_played_activity.xml``) + - ``recently_played_fragment_ongoing_story_recycler_view`` (a ``RecyclerView`` in ``recently_played_fragment.xml``) + +## build.gradle file +- Arrange lists in alphabetical order unless there's a good reason not to. +- Combine `implementation`, `androidTestImplementation` and `testImplementation` to declare all similar dependencies in one block. \ No newline at end of file diff --git a/wiki/Contributing-to-Oppia-Android.md b/wiki/Contributing-to-Oppia-Android.md new file mode 100644 index 00000000000..9c184e4cb26 --- /dev/null +++ b/wiki/Contributing-to-Oppia-Android.md @@ -0,0 +1,103 @@ +_These instructions are for developers who'd like to contribute code to improve the Oppia platform. If you'd prefer to help out with other things, please see our [general contribution guidelines](https://github.com/oppia/oppia-android/wiki)._ + +Thanks for your interest in contributing to the Oppia Android project, and making it easier for students to learn online in an effective and enjoyable way! + +If you run into any problems along the way, we're here to help! Check out our [wiki page on getting help](https://github.com/oppia/oppia-android/wiki/Get-Help) for the communication channels you can use. If you find any bugs, you can also file an issue on our [issue tracker](https://github.com/oppia/oppia-android/issues). There are also lots of helpful resources in the sidebar, check that out too! + +**Important! Please read this page in its entirety before making any code changes.** It contains lots of really important information. You should also read through our [guide to making pull requests](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR). + +## Table of Contents + +- [Onboarding instructions](#onboarding-instructions) + - [Guidance on submitting a PR](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR) + - [Developing your skills](https://github.com/oppia/oppia-android/wiki/Developing-skills) +- [Important: Ongoing Bazel migration](#important-ongoing-bazel-migration) +- [Installing the Oppia web app](#installing-the-oppia-web-app) +- [Communication channels](#communication-channels) + +## Onboarding instructions + +If you'd like to help out with the Android project, please follow the following steps to get started: + +### 1. Complete the preliminaries + +- Sign the CLA, so that we can accept your contributions. If you're contributing as an individual, use the [individual CLA](https://goo.gl/forms/AttNH80OV0). If your company owns the copyright to your contributions, a company representative should sign the [corporate CLA](https://goo.gl/forms/xDq9gK3Zcv). + +- Fill in the [Oppia contributor survey](https://goo.gl/forms/otv30JV3Ihv0dT3C3) to let us know your interests. (You can always change your responses later.) **Make sure to indicate prominently that you are interested in helping out with Android.** + +- Say hi and introduce yourself on [GitHub Discussions](https://github.com/oppia/oppia-android/discussions/4788)! + +### 2. Set up your development environment + +- Follow the instructions on [Installing Oppia Android](https://github.com/oppia/oppia-android/wiki/Installing-Oppia-Android) to prepare your developer environment and install Oppia Android. **Note:** Make sure you have good Internet connectivity when developing on Oppia Android, since this project uses third party libraries which are needed to build the app. + +- Familiarize yourself with the resources linked to from the wiki sidebar, especially the [overview of the codebase](https://github.com/oppia/oppia-android/wiki/Overview-of-the-Oppia-Android-codebase-and-architecture), the [coding style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide), and the [Frequent Errors and Solutions](https://github.com/oppia/oppia-android/wiki/Frequent-Errors-and-Solutions). (You don't have to read everything else right now, but it's a good idea to be aware of what's available, so that you can refer to it later if needed.) + + +### 3. Pick your first starter issue! + +We suggest choosing your first issue from the [list of good first issues](https://github.com/oppia/oppia-android/labels/good%20first%20issue). These issues are hand-picked to ensure that you don't run into unexpected roadblocks while working on them, and each of them should have clear instructions for new contributors. (If not, please let us know via [GitHub Discussions](https://github.com/oppia/oppia-android/discussions) and we'll try to fix it.) + +When you've found an issue you'd like to tackle: + +- Leave a comment that describes in detail how you'll tackle it (e.g. explain which file(s) you would modify and what changes you would make), and @-mention the team lead (typically **@BenHenning**). If your explanation makes sense, we'll assign the issue to you. +- Submit a PR, following the [guidance on submitting a PR](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR). (Consider bookmarking this guide in your browser for easy reference. We also suggest bookmarking the ["my issues" page](https://github.com/issues/assigned) so that you can keep track of the issues assigned to you.) +- If you run into any problems, feel free to create a [GitHub Discussion](https://github.com/oppia/oppia-android/discussions) and get help from the Oppia community, or [request a mentor](https://forms.gle/udsRP4WQgLcez9Zm8) if you'd like individual support. + +You are also welcome to tackle non-starter issues, but for those, you might need to be more independent, because we might not know how to solve them either! + +Once you've merged PRs that correspond to **two** different pre-existing GitHub issues, please fill in [this form](https://forms.gle/NxPjimCMqsSTNUgu5) to become an Oppia collaborator! This will grant you access to the repository, and allow you to join a team. (But please don't create your own issues and then make PRs for them -- that won't count.) + + + +## Important: Ongoing Bazel migration + +The team is currently using two build systems for the project: Gradle and Bazel. We're in the process of actively migrating to Bazel. + +Please note that: +- It's currently recommended that all team members use **Gradle** for their active development in Android Studio. While some team members use the Bazel Android Studio plugin instead of Android Gradle Plugin (AGP), we make this recommendation because day-to-day Bazel development currently suffers from: + - Significant memory overhead that continues to grow without careful pruning (i.e. periodic shutdowns of the local Bazel build server). On some Linux distros, this can result in a Kernel panic when memory is fully exhausted. + - Various symbolic errors throughout the codebase that can make it much more difficult to jump to specific symbols (though, unlike Gradle, all code including scripts are editable and runnable within Android Studio). +- That said, when submitting a PR for review, you may notice that some Bazel-specific tests or workflows fail. Investigating and fixing these will require setting up Bazel in your local environment (see the instructions [here](https://github.com/oppia/oppia-android/wiki/Oppia-Bazel-Setup-Instructions)), and then running the specific Bazel commands in your local repository (most team members just use the console within Android Studio to run their Bazel commands). +- Bazel & Gradle sometimes don't play nicely with one another. So, when you're verifying Bazel-specific things, we recommend doing so in one go, and then deleting the corresponding Bazel build artifacts using ``bazel clean`` before switching back over to Gradle (to avoid any issues with the two build systems crossing). Note that Bazel generally doesn't have any problems with Gradle build artifacts, so there's no need to clean the Gradle project first. +- As the team finishes the migration to Bazel, communications and instructions will be sent ahead of time for moving development environments away from Gradle so that we can officially deprecate it. + +## Installing the Oppia web app + +If you need to connect to a local version of the Oppia web application, check out a copy of the [Oppia web app repository](https://github.com/oppia/oppia) and get it running locally. This will allow you to connect to a local instance of the web app when developing locally. + +For now, you generally won't need to do this, until the Android app supports on-the-fly lesson downloading (which we're currently working on). + + +### Notes + +* Our central development branch is `develop`, which should be clean and ready for release at any time. All changes should be done in feature branches based off of `develop`. + +* To find the author of a particular change in a file, run this command: + + ``` + git blame file-name + ``` + The output will show the latest commit SHA, author, date, and time of commit for each line. + + To confine the search of an author between particular lines in a file, you can use: + + ``` + git blame -L 40,60 file-name + ``` + The output will then show lines 40 to 60 of the particular file. + + For more `git blame` options, you can visit the [git blame documentation](https://git-scm.com/docs/git-blame). + +* If your PR includes changing the location of the file, if you simply move the file by cut and paste method, then the git will track it as a new file. So to prevent this, use: + ``` + git mv old_file_path new_file_path + ``` + By using this command git will detect the file as a renamed file. + + +## Communication channels + +If you run into any problems (including non-coding ones), you can ask questions on [GitHub Discussions](https://github.com/oppia/oppia-android/discussions). You can use this avenue for asking anything -- questions about any issue, who to contact for specific things, etc. + +You can also check out the [developer mailing list](https://groups.google.com/forum/?fromgroups#!forum/oppia-android-dev). \ No newline at end of file diff --git a/wiki/Creating-a-screenshot.md b/wiki/Creating-a-screenshot.md new file mode 100644 index 00000000000..abba5a08f7d --- /dev/null +++ b/wiki/Creating-a-screenshot.md @@ -0,0 +1,44 @@ +There are three ways in which we can easily take screenshots and get it on the computer + +## Using Android Studio on an Emulator +If you are running Oppia on an emulator running on Android Studio, you can directly take a screenshot from the sidebar of the emulator. +

+Screen Shot 2019-11-08 at 3 58 05 PM +

+ +Also, you can create a video/gif file from emulator by following these steps: +1. Click on three dots present at the bottom on emulator side bar. +2. In the extended menu click on `Record and Playback`. +3. Click on `Start Recording`. This will only record the emulator screen. +4. Once finished, click on `Stop Recording`. +5. You can save this file in `WEBM` or `GIF` format. + +The default location for the screenshot, video and gif would be on Desktop. + +## Using Android Studio on a local device +If you are running Oppia on a local Android device, you can take a screenshot using the Logcat window of Android Studio. +* Go to Tools > Windows > Logcat. You will see the Logcat window open up at the bottom. +* On the left hand side there is a column of icons, hover on the expand icon at the bottom and you will see a camera icon: . + +Screenshot 2021-11-09 at 7 54 36 PM + +* Click on that and Android studio will take a screenshot and open up a Screenshot Editor. + +Screenshot 2021-11-09 at 7 50 53 PM + +* Optionally change the image: +> * **Recapture**: Take a new screenshot. +> * **Rotate Left**: Rotate the image 90 degrees counter-clockwise. +> * **Rotate Right**: Rotate the image 90 degrees clockwise. +> * **Frame Screenshot**: Choose a device to wrap your screenshot with real device artwork. +* Click Save, specify the location and filename, and then click OK to save the screenshot. + + +For more details, see the [Android documentation](https://developer.android.com/studio/debug/am-screenshot) + +## Using an ADB command +If you would like to take a screenshot but cannot use Android Studio, you can instead capture a screenshot using an ADB command. +* Run the following command: `adb shell screencap -p /sdcard/screenshot.png` +* This creates a screenshot called screenshot.png and stores it on your Android device. +* To pull it on to your computer, you can run `adb pull /sdcard/screenshot.png`. This will copy the image from the device to your local machine (on the path in which you ran the command). +* You can now upload the image on to Github. \ No newline at end of file diff --git a/wiki/Dagger.md b/wiki/Dagger.md new file mode 100644 index 00000000000..c92830d9366 --- /dev/null +++ b/wiki/Dagger.md @@ -0,0 +1,132 @@ +# What is Dagger? +Dagger is a fully static and compile-time [dependency injection](https://github.com/oppia/oppia-android/wiki/Dependency-Injection) framework. Compile-time means that issues in the dependency graph (such as cycles or missing providers) are caught during build-time. + +## Components & Subcomponents +Dagger creates the dependency graph using components and subcomponents +- Components are top-level containers of providers that are pulled from modules that component is configured to include +- Subcomponents are also containers, and may contain other subcomponents +- Subcomponents automatically inherit all the dependencies from their parent components +- Components/subcomponents can automatically collect dependencies for which they are scoped + +## Scopes +Scopes are compile-time annotations associated both with a component/subcomponent and either injectable objects or providers of objects + +## Injectables & Providers +Dagger supports two types of injections: field and constructors +- All objects that are injected need to have an @Inject-declared constructor +- Any parameters passed into an @Inject-declared constructor will be retrieved from the dependency graph +- Fields can be marked as @Inject-able, but a separate inject() method in a component needs to be added for that class to initialize those fields, and the class must call this method + +Note: Classes can have their providers inferred just by being qualified and having an @Inject-able constructor--no need for a Dagger module + +## Modules +Dagger modules are defined in separate classes annotated with the @Module tag +- Modules can provide an implementation with @Provides +- Modules can bind one type to another type using @Binds + +# Its Usage in Oppia Android + - Dagger object lifetimes need to be compatible with Android object lifecycles + - Prefer constructor injection over field injection to encourage encapsulation + - Result are activity/fragment/view presenter classes that are field-injected into their corresponding Android objects, but themselves support constructor injection + + + + + + +
+ There's an Android-specific dependency hierarchy: +
    +
  • Root application component for @Singleton that's initialized in a custom Application class
    • +
  • Per-activity and per-fragment subcomponents that are initialized in base activity and fragment classes, respectively
    • +
  • Inheritance such that all application-level classes can be injected in activities, and all activity-level classes can be injected into fragments (but not vice versa for either)
    • +
  • Activity & fragment controllers for each activity and fragment that are @Inject-constructed, and can inject all needed dependencies and perform necessary UI logic
    • +
  • Activity and fragment classes become boilerplate classes that extend base classes that enable DI for them, and delegate UI callbacks to their controller classes for processing
    • +
  • View models are also injectable for use in controllers per a ProAndroidDev article
    • +
  • Similar to Dagger Android, but with better encapsulation
    • +
+ 		+ +
+ +You can understand it with this example : + +This is a Singleton-scoped object with dependency. Note that because Factory is `@Singleton` scoped, it can inject everything in the Singleton component including blocking dispatcher. +``` +@Singleton +class Factory @Inject constructor(@BlockingDispatcher private val blockingDispatcher: CoroutineDispatcher) { + fun create(): InMemoryBlockingCache { + return InMemoryBlockingCache(blockingDispatcher) + } +} +``` + +These are Singleton-scoped providers with custom qualifiers. Note also that to distinguish between two of the same types, we can use custom qualifier annotations like @BackgroundDispatcher and @BlockingDispatcher. + +``` +@Module +class DispatcherModule { + @Provides + @BackgroundDispatcher + @Singleton + fun provideBackgroundDispatcher(): CoroutineDispatcher { + return Executors.newFixedThreadPool(4).asCoroutineDispatcher() + } + + @Provides + @BlockingDispatcher + @Singleton + fun provideBlockingDispatcher(): CoroutineDispatcher { + return Executors.newSingleThreadExecutor().asCoroutineDispatcher() + } +} +``` + + + +# How to write Tests with Dagger +- Dependencies can be replaced at test time +- This is especially useful for API endpoints! We can replace Retrofit instances with mocks that let us carefully control request/response pairs +- This is also useful for threading! We can synchronize coroutines and ensure they complete before continuing test operations +- Tests can declare their own scoped modules in-file +- Tests themselves create a test application component and inject dependencies directly into @Inject-able fields +- Bazel (#59) will make this even easier since test modules could then be shareable across tests + +Here is an example of testing with Oppia Dagger. This shows setting up a test component and using it to inject dependencies for testing purposes. It also shows how to create a test-specific dependency that can be injected into a test for manipulation. +``` +class InMemoryBlockingCacheTest { + @ExperimentalCoroutinesApi @Inject @field:TestDispatcher lateinit var testDispatcher: TestCoroutineDispatcher + @ExperimentalCoroutinesApi private val backgroundTestCoroutineScope by lazy { CoroutineScope(backgroundTestCoroutineDispatcher) } + @ExperimentalCoroutinesApi private val backgroundTestCoroutineDispatcher by lazy { TestCoroutineDispatcher() } + + @Before @ExperimentalCoroutinesApi fun setUp() { setUpTestApplicationComponent() } + + @Test @ExperimentalCoroutinesApi fun `test with testDispatcher since it's connected to the blocking dispatcher`() = runBlockingTest(testDispatcher) { /* ... */ } + + private fun setUpTestApplicationComponent() { + DaggerInMemoryBlockingCacheTest_TestApplicationComponent.builder().setApplication(ApplicationProvider.getApplicationContext()).build().inject(this) + } + + @Qualifier annotation class TestDispatcher + @Module + class TestModule { + @ExperimentalCoroutinesApi @Singleton @Provides @TestDispatcher fun provideTestDispatcher(): TestCoroutineDispatcher { return TestCoroutineDispatcher() } + @ExperimentalCoroutinesApi @Singleton @Provides @BlockingDispatcher + fun provideBlockingDispatcher(@TestDispatcher testDispatcher: TestCoroutineDispatcher): CoroutineDispatcher { return testDispatcher } + } + + @Singleton + @Component(modules = [TestModule::class]) + interface TestApplicationComponent { + @Component.Builder interface Builder { @BindsInstance fun setApplication(application: Application): Builder fun build(): TestApplicationComponent } + fun inject(inMemoryBlockingCacheTest: InMemoryBlockingCacheTest) + } +} +``` + +# Points to Note +Dagger compile-time errors can be hard to understand +- When you encounter one: scan the error for the dependency name (it's likely a dependency you just imported into the file failing to compile) +- Search for the Dagger module you want to use to provide that dependency +- Make sure your Gradle module or Bazel build file depends on the library that contains the module you need +- Note that Gradle modules cannot depend on the app module, which means any Dagger modules in the app Gradle module are inaccessible outside of the app module \ No newline at end of file diff --git a/wiki/Dark-Mode.md b/wiki/Dark-Mode.md new file mode 100644 index 00000000000..87a6e95bf87 --- /dev/null +++ b/wiki/Dark-Mode.md @@ -0,0 +1,98 @@ +_Author credit: Thanks to **@ayush0402** for writing up the initial version of this guide._ + +## Overview +This guide explains the newly adopted convention for using colors in oppia-android and adding support for dark mode +to any particular layout while keeping the code organised and strictly following the decided convention. + +The approach is to split color declarations in 4 different files instead of keeping the colors at one place, promoting separation of +concerns and hence improving code maintainability and understandability. + + + +#### Knowing the convention + +The following files have been added for maintaining the colors : +1. **`color_defs.xml`**
+ This file should strictly contain actual color names (irrespective of their intended use) with their hex color code declaration.
+ example:
+ > Don't + ```xml + #90EE90 + #90EE90 + #FF0000 + ``` + > Do + ```xml + #90EE90 + #0000FF + ``` + Declarations from this file should be only used in `color_palette.xml`. + +2. **`color_palette.xml`**
+ There are two versions for this file (day/night variations). The purpose of this file is to split the colors for them to be later referenced by `component_colors.xml`. The color declarations in this should have a generic name and should not contain any name tied to the intended component like toolbar, edittext, textview, etec. to be used on. Also, colors should **not** contain "feature" based name (like *add_profile_background_color*). + The declarations in this file should only reference `color_defs.xml`. + >Don't: + ```xml + @color/light_black + @color/oppia_pink + ``` + >Do: + ```xml + @color/light_black + @color/oppia_pink + ``` + You can refer to both variations of these files to see how it separates the colors. +3. **`component_colors.xml`**
+ This file contains the highest level of color declarations. The declarations in this file should only reference `color_palette.xml`. It uses UI component specific names. Component colors should be shared very little outside of their respective views/fragments/activities. *All the layouts/views should only reference this file for colors.*
+ examples:
+ ```xml + @color/primary_text_color + @color/toolbar_color + + @color/primary_text_color + @color/primary_text_color + @color/text_input_background_color + + @color/description_text_color + @color/background_color + + @color/primary_text_color + @color/dark_text_color + ``` +4. **`colors_migrating.xml`**
+ This file contains color declarations which are supposed to be in color_defs.xml but has not been renamed yet to have actual color name instead of names linked to their use and components. This is a temporary measure to make sure other 4 color files follows the convention decided for them. + This file should be deleted after all colors have been shifted to `color_defs.xml`.
+ +_Note:_ +- *All color names should strictly follow `snake_case` naming convention.*
+- *All colors in `component_colors.xml` and `color_palette.xml` should have `_color` as suffix and just the opposite for `color_defs.xml`*
+- *All color declaration must have their parent file name as prefix of their name, i.e. "`_`" (Look at the color name examples for better understanding.)* + + +

+visual representation of color hierarchy

+ +## Working with the layouts +Currently most of the layouts are directly referencing colors from `color_defs.xml`, they don't have separate colors for day and night mode. Our goal here is to make sure that views and layouts are using specified colors for day and night wherever applicable. + +You can refer to the design mocks for expected final result : [Dark Mode Mocks](https://xd.adobe.com/view/c05e9343-60f6-4c11-84ac-c756b75b940f-950d/grid/) + +#### How to achieve this goal? +Here is how I would go around working with any particular layout...
+ +- Replace all the generic colors in the layout with something more specific to the component by defining it in the `component_colors.xml`, generally it should be named in the format *`component_color___color`*. + +- Go through the mock for the concerned activity and note down which component of the app needs separate colors for day and night modes. The mock has provided hex color codes for all the elements in the UI, if any of the colors is not already present in the `color_defs.xml` then add it to the file with the actual color name. + +- Now, the newly defined colors in `component_colors.xml` should reference to something in `color_palette.xml`, define new colors in `color_palette.xml` based on general use case if not already defined. You will need to define same colors twice, in `values\color_palette.xml` as well as `values-night\color_palette.xml`. Both these declarations can be same as well, if there is no difference in the mocks for day and night mode. Make sure `color_palette.xml` is using colors from `color_defs.xml` only. + +Naming these colors can be bit tricky so it is suggested to take help from already exisitng colors in these files. + +In short, the general idea is to make sure layouts reference colors only from `component_colors.xml`, which is then referencing a version of `color_palette.xml` based on the active theme, making sure all the color declarations are as per the conventions decided for them. + +*Tip: Use layout Inspector to know more about targeted views.* + +### Running the app with dark mode +We suggest running the app on an API 30 Google Pixel emulator using the system-wide dark mode option from settings.
+ +Some other skins of android might force their own version of dark mode to screens not having dark mode support yet. diff --git a/wiki/DataProvider-&-LiveData.md b/wiki/DataProvider-&-LiveData.md new file mode 100644 index 00000000000..a3da67cef61 --- /dev/null +++ b/wiki/DataProvider-&-LiveData.md @@ -0,0 +1,47 @@ +### DataProviders + +[DataProvider](https://github.com/oppia/oppia-android/blob/a85399c2b0a2b9cf214881ce8c70d9b487f1e0b8/utility/src/main/java/org/oppia/android/util/data/DataProvider.kt#L10)s are gateways to safely receiving an asynchronous result from an operation. They support notifications for when the DataProvider has new data to be retrieved, force usage of suspend functions to encourage coroutine use, have utilities for simplifying their usage, and provide an easy way to pass data to the UI via LiveData. + +You should generally never need to create a new DataProvider since there are existing bridges for most asynchronous operations, but if you do make sure to follow other DataProviders for a reference to make sure you're implementing it correctly. + + +### Transferring data to UI via LiveData + +[``LiveData``](https://developer.android.com/topic/libraries/architecture/livedata) is a lifecycle-aware stateful concurrency primitive that was added to Android Jetpack. The team prefers using ``LiveData`` for a few reasons: +1. It supports receiving data from a background thread via an Android ``Handler`` post +2. It's lifecycle-aware (e.g. it ensures that the background data passed from (1) does not trigger logic for an activity that's being torn down due to the user exiting it or a configuration change) +3. It integrates trivially with Android [databinding](https://developer.android.com/topic/libraries/data-binding) which the team uses to simplify UI development + +All ``DataProvider``s are convertible to ``LiveData`` using an extension function: [``toLiveData()``](https://github.com/oppia/oppia-android/blob/a85399c2b0a2b9cf214881ce8c70d9b487f1e0b8/utility/src/main/java/org/oppia/android/util/data/DataProviders.kt#L158). + +### Best practices/antipractices + +- Prefer to start with a ``DataProvider`` (e.g. in-memory, file-provided, or network-provided) and then transform/combine it as needed rather than creating new dispatchers +- Never use coroutines outside of the domain layer +- Never perform any multi-threading operations outside of coroutines (except when unavoidable--see the 'other cases for background processing' section) +- Never use locks within coroutines +- Prefer using concurrent data structures over atomics +- Never send data to the UI without using a ``DataProvider`` + ``LiveData`` +- **Do** make use of ``TestCoroutineDispatchers`` when writing tests to ensure proper synchronization +- When integrating a new library that has asynchronous support, make sure it's configurable and that its executors/dispatchers are set up to use the common dispatchers +- Prefer using existing ``DataProvider``s rather than creating new ones +- Never use ``observeForever`` on a ``LiveData`` in the UI, and in cases when it's used elsewhere make sure the observer is removed +- Prefer conducting transformations in ``DataProvider``s rather than ``LiveData``, except when impossible (e.g. extracting values from the final ``AsyncResult`` passed from the data provider) +- Never combine data through ``LiveData`` (e.g. using ``MediatorLiveData``); prefer combining data through ``DataProvider``s instead and convert to ``LiveData`` at the end +- Never use ``SharedPreferences``--use ``PersistentCacheStore`` instead since it never blocks the main thread + +## DataProvider simplifications + +There are a number of preexisting ``DataProvider``s & utilities to simplify development. + +### DataProviders utility + +[``DataProviders``](https://github.com/oppia/oppia-android/blob/a85399c2b0a2b9cf214881ce8c70d9b487f1e0b8/utility/src/main/java/org/oppia/android/util/data/DataProviders.kt#L24) is an injectable class that provides a number of helpful functions: +- Transforming data providers (e.g. converting their type into a new type, or performing some operation once data is available) +- Combining two or more data providers together (which will block on all data providers being ready) +- Converting data providers to ``LiveData`` +- Creating an in-memory data provider (to, for example, start a data provider chain) + +### LiveData transformations + +``LiveData`` supports [``Transformations``](https://developer.android.com/reference/androidx/lifecycle/Transformations) & other utilities (like ``MediatorLiveData``) which can be used to transform and/or combine ``LiveData`` objects similarly to ``DataProvider``s. The team generally makes use of transformations to perform boundary error checking/value retrieval from ``AsyncResult``, but otherwise prefers to leverage ``DataProvider``s for more complex combines/transformations. ``LiveData``'s utilities have some shortcomings and are fairly easy to get wrong, so prefer to use ``DataProvider``s when possible. diff --git a/wiki/Dependency-Injection.md b/wiki/Dependency-Injection.md new file mode 100644 index 00000000000..313d7a885f6 --- /dev/null +++ b/wiki/Dependency-Injection.md @@ -0,0 +1,52 @@ +# What is Dependency Injection? +- It's a mechanism to automatically provide dependencies that an object depends on +- It requires the object receiving dependencies to also be injectable for other dependencies +- The dependency injection (DI) framework is responsible for object lifetimes +- Modules are classes responsible for providing providers +- Providers act like factories that create new instances of a dependency +- The result is a graph of dependencies and providers +- This graph must be acyclic, otherwise it cannot be formed (e.g. dependency A cannot inject B if B depends on C and C depends on A) + +# Why do we need it? +Consider this scenario for typical dependency situation + +- In this example, creating a UserAppHistoryController requires creating a PersistentCacheStore factory +- Creating the factory requires a context object +- Why should users of the controller care about factories and contexts? +- This situation is simpler than real situations typically are +- Wouldn't it be nice if those dependencies could just magically show up? + +``` +class UserAppHistoryController(cacheStoreFactory: PersistentCacheStore.Factory) { + // Use cacheStoreFactory... +} + +class PersistentCacheStore private constructor() { + class Factory(private val context: Context) { + // Use context... + } +} +``` + +Now if we would have introduced dependency injection + +- We just need to inject UserAppHistoryController wherever we want it, and not care about dependencies +- However, using one injected dependency requires using another object that's injected, and so on +- With a DI framework, essentially the entire app needs to have injected dependencies + +``` +@Singleton +class UserAppHistoryController @Inject constructor( + cacheStoreFactory: PersistentCacheStore.Factory +) { + // Use cacheStoreFactory... +} + +class PersistentCacheStore private constructor() { + class Factory @Inject constructor (private val context: Context) { + // Use context... + } +} +``` + +Hence this calls for a framework that can help us with Dependency Injection in Android and Dagger seems the most favourable choice. [ReadMore](https://github.com/oppia/oppia-android/wiki/Dagger) \ No newline at end of file diff --git a/wiki/Developing-skills.md b/wiki/Developing-skills.md new file mode 100644 index 00000000000..562b07e321e --- /dev/null +++ b/wiki/Developing-skills.md @@ -0,0 +1,20 @@ +In general, it's easier to contribute to the Oppia codebase if you have some knowledge of git, as well as at least one of Kotlin or Android. You don't need to know all of this before you start, though! Many of our contributors have picked these skills up concurrently while tackling their first issues. + +That said, we strongly recommend that you be open to learning new things. If you need to brush up on some of the technologies used in Oppia, here are some resources that may help: + + - Git and Github are used to make changes to the repository, so it's good to know how to use them to do basic stuff like branching, merging, pulling, pushing, and committing. + - For beginners: + - [Learning Branching Git](https://learngitbranching.js.org/) helps explain how git works. Try the levels below: + - Levels 1, 2, and 3 from the Introduction sequence. + - Levels 1, 2, 3, 4, 5, and 6 from Push and Pull Git Remotes. + - [Introduction to GitHub](https://lab.github.com/githubtraining/introduction-to-github) covers how to use GitHub. + - More advanced: + - The other levels from [Learn Branching Git](https://learngitbranching.js.org/) cover git in more depth. + - You may find this [git visualizer](https://git-school.github.io/visualizing-git/) helpful for understanding more advanced git operations. It can be helpful for simple ones too! + - GitHub's [managing merge conflicts page](https://lab.github.com/githubtraining/managing-merge-conflicts) explains how to address merge conflicts. + - Kotlin is used for Android in oppia. You can learn the basics of kotlin from Udacity -- [Kotlin bootcamp for programmers](https://www.udacity.com/course/kotlin-bootcamp-for-programmers--ud9011) by Google. + - Learn the basics of android to understand the project structure and the libraries that are used in most common apps from the Udacity -- [Developing Android Apps with Kotlin](https://www.udacity.com/course/developing-android-apps-with-kotlin--ud9012) course. + - To learn the advanced topics like Dependency Injection and Testing in Android check out Udacity -- [Advanced Android with Kotlin](https://www.udacity.com/course/advanced-android-with-kotlin--ud940) course. + + +All the learning resources mentioned are free. If you find a useful learning resource that isn't listed here, please add it to help other new contributors! diff --git a/wiki/End-to-End-Testing-Guide.md b/wiki/End-to-End-Testing-Guide.md new file mode 100644 index 00000000000..63e86fcf831 --- /dev/null +++ b/wiki/End-to-End-Testing-Guide.md @@ -0,0 +1,111 @@ +## Overview + +End-to-End tests test the app from an end user’s experience by simulating the real user scenario and validating the system under test and its components for integration and data integrity. + +These tests play a major role in publishing the app. They run on a real or emulated device to make sure that our code interacts with the Android environment as expected, providing confidence in the final application or a feature when it's finished. + +End-to-End tests in Oppia-android are written using [UiAutomator](https://developer.android.com/training/testing/ui-automator). +These tests are written under the instrumentation module and don’t have Gradle support. + +``` +instrumentation/ -- android test binaries for each test suite. +`-- src + |-- java + | `-- org + | `-- oppia + | `-- android + | `-- instrumentation + | `-- application -- Test application and modules + `-- javatests + `-- org + `-- oppia + `-- android + `-- instrumentation -- Test suites for each part of the app + +``` + +These tests are run using [Bazel](https://bazel.build/) and [ADB](https://developer.android.com/studio/command-line/adb). +Each test suite tests a particular part of the app, each test suite has its own [kt_android_library](https://bazelbuild.github.io/rules_kotlin/kotlin#kt_android_library), [android_binary](https://docs.bazel.build/versions/main/be/android.html#android_binary) and [android_instrumentation_test](https://docs.bazel.build/versions/main/be/android.html#android_instrumentation_test). + +Note: android_instrumentation_test target is not supported yet ([#3617](https://github.com/oppia/oppia-android/issues/3617) for details). + +### How it works. +The android_binary of a test suite generates a test apk with the same name as the class of the test suite. This test apk is installed along with the original apk in the emulator. We use the adb [am instrument](https://developer.android.com/studio/test/command-line#AMSyntax) command to run the test on the target device or emulator. + +### How to run an End-to-End test + +#### Prerequisites: +1. [Set up Bazel for Oppia](https://github.com/oppia/oppia-android/wiki/Oppia-Bazel-Setup-Instructions#installation) +2. Add adb to the environment (platform-tools) i.e, add the following line to the .bashrc or the file path (note that this is platform dependent): + ``` + export PATH=/home//Android/Sdk/platform-tools:$PATH + ``` +3. download and install [test-services-1.1.0.apk](https://mvnrepository.com/artifact/androidx.test.services/test-services/1.1.0) and [orchestrator-1.1.0.apk](https://mvnrepository.com/artifact/androidx.test/orchestrator/1.1.0) in the emulator. (run command in the directory where both apk are downloaded) + ``` + adb install -r test-services-1.1.0.apk && adb install -r orchestrator-1.1.0.apk + ``` + +4. java version 8 (Optional, only for uiautomatorviewer) + ``` + java -version + ``` + *output:* + openjdk version "1.8.0_292" + OpenJDK Runtime Environment (build 1.8.0_292-8u292-b10-0ubuntu1~18.04-b10) + OpenJDK 64-Bit Server VM (build 25.292-b10, mixed mode) + +#### Steps to run the tests + +1. Build the oppia_test and test suite android_binary from the **root** repository directory: + ``` + bazel build //instrumentation:oppia_test && bazel build //instrumentation: + ``` + e.g.: + ``` + bazel build //instrumentation:oppia_test && bazel build //instrumentation/src/javatests/org/oppia/android/instrumentation/player:ExplorationPlayerTestBinary + ``` +2. install the oppia_test.apk and the test suite’s APK + ``` + adb install -r bazel-bin/instrumentation/oppia_test.apk && adb install -r bazel-bin/instrumentation/.apk + ``` + e.g.: + ``` + adb install -r bazel-bin/oppia_test.apk && adb install -r bazel-bin/instrumentation/src/javatests/org/oppia/android/instrumentation/player/ExplorationPlayerTestBinary.apk + ``` +3. Run the instrumentation tests using am instrument command + ``` + adb shell 'CLASSPATH=$(pm path androidx.test.services) app_process / \ + androidx.test.services.shellexecutor.ShellMain am instrument -w -e clearPackageData true \ + -e targetInstrumentation org.oppia.android.app.instrumentation/androidx.test.runner.AndroidJUnitRunner \ + androidx.test.orchestrator/.AndroidTestOrchestrator' + ``` + **Note:** "-e clearPackageData true" is used to clear your app's data in between runs. + +### Best practices when writing End-to-End tests +1. Each test suite should use the macro [oppia_instrumentation_test](https://github.com/oppia/oppia-android/blob/develop/instrumentation/oppia_instrumentation_test.bzl#L7) which generates the necessary targets required for each test suite. +2. Tests should follow the best practices for [writing good tests in Oppia Android](https://github.com/oppia/oppia-android/wiki/Oppia-Android-Testing), including ensuring that tests are focused on a single, high-level behavior that the suite wishes to verify as working (such as being able to play fully through an exploration). +3. Repetitive actions should be factored into helper methods. When these are generally useful for other end-to-end tests, they should be moved to [EndToEndTestHelper](https://github.com/oppia/oppia-android/blob/854071ab6adec35192be6d517ae16d2f3300ebb0/instrumentation/src/java/org/oppia/android/instrumentation/testing/EndToEndTestHelper.kt). +4. Prefer using existing helpers from ``EndToEndTestHelper`` rather than reimplementing them. +5. Prefer testing whole end-to-end flows rather than specific behaviors. For example: opening the app, downloading a topic, and being able to play it is an excellent end-to-end test since it’s verifying multiple cross-stack behaviors. Conversely, testing that a thumbnail is aligned correctly on the home screen is less useful and ought to be tested in Robolectric or Espresso local to the component showing that image. +6. Use uiautomatorviewer to get details of each view such as resource ID, content description, class, and other properties. + +### Writing E2E tests +Unlike Robolectric and Espresso, tests in UiAutomator don't share the same code as Espresso and Robolectric. UiAutomator tests are dependent on [UiDevice](https://developer.android.com/reference/androidx/test/uiautomator/UiDevice). UiDevice provides access to all the views and gives possibilities to stimulate all UserActions in the device/emulator including an app other than the current app. + +In the instrumentation module all the UiAutomator tests are written using the Extensions of UiDevice in [EndToEndTestHelper.kt](https://github.com/oppia/oppia-android/blob/develop/instrumentation/src/java/org/oppia/android/instrumentation/testing/EndToEndTestHelper.kt) + +**Example:** +To navigate from Profile screen to a Exploration page +``` + // device is a Instance of UiDevice. + device.findObjectByRes("skip_text_view").click() // Click on the "skip" button on the onBoarding page. + device.findObjectByRes("get_started_button").click() // Click on the "Getting Started' button. + device.waitForRes("profile_select_text") // Waiting for the Profile Select screen to appear. + device.findObjectByText("Admin").click() // Click on the admin profile. + scrollRecyclerViewTextIntoView("First Test Topic") // Scroll to the view with text "First Test Topic. + device.findObjectByText("First Test Topic").click() // Click on the "First Test Topic" text. + device.findObjectByText("LESSONS").click() // Click on the "Lessons" tab. + device.findObjectByText("First Story").click() // Click on the "First Story" tab. + scrollRecyclerViewTextIntoView("Chapter 1: Prototype Exploration") // Scroll to first exploration. + device.findObjectByText("Chapter 1: Prototype Exploration").click() // Click on the first exploration. +``` \ No newline at end of file diff --git a/wiki/Firebase-Console-Guide.md b/wiki/Firebase-Console-Guide.md new file mode 100644 index 00000000000..e25da5475c1 --- /dev/null +++ b/wiki/Firebase-Console-Guide.md @@ -0,0 +1,114 @@ +This document is made as a guide for viewing and customising the Firebase project(s) of Oppia-Android. It is recommended to fully read this doc before making any changes to the Firebase project. If you only want to view the Firebase console then this doc can help you in viewing exactly the thing that you want to see. It must also be noted that appropriate permissions are required for viewing/editing anything in the console. For permissions, please contact oppia-android-dev@googlegroups.com. + +## Table of Contents +* [Viewing the Console](#viewing-the-console) + * [Crashlytics](#crashlytics) + * [Analytics](#analytics) + * [DebugView](#debugview) + * [Funnels](#funnels) + * [Adding Custom Parameters](#adding-custom-parameters) + * [Conversions](#conversions) +* [Your own Firebase Project](#your-own-firebase-project) + * [Setting it up](#setting-it-up) + * [App registration](#app-registration) +* [View events in the Android Studio debug log](#view-events-in-the-android-studio-debug-log) + +## Viewing the Console +* Go to the [Firebase Console](https://console.firebase.google.com) -> Log in to your gmail account (if you haven't) -> Select Oppia Android Dev Project. +* You will be welcomed by the following screen : +> + +### Crashlytics +* Scroll down the left nav bar and select Crashlytics. You will see the following dashboard : +> +* A --> Filter Option : You can filter out crashes and non-fatal exceptions through it. +* B --> Crash-free stats : Give you an insight to the amount of users which are experiencing crash free experience. +* C --> Event trends : Give you the information regarding the reported exceptions and the users affected by them. +* D --> Issues : Similar reported exceptions are clubbed together to form issues which can be marked as solved once fixed. Once we click on any one of these issues, it leads us to the following : +> +* A --> Events by version : This segment tells the user about the day-wise number of event occurrences in form of a graph. This graph may have multiple lines based on the number of versions of the application. +* B --> Device Info : It tells us about the devices in which that exception has occurred. Multiple devices are differentiated on the basis of the number of event occurrences. Other info like the OS and the device states are also available. +* C --> Session Summary : This segment contains the crash reports of each occurrence of the event. Things like stack traces and device data are present which tell us a lot about the crashes. Keys and Logs are only available if we log them through APIs. + +### Analytics +* Scroll down the nav bar a bit more and select Dashboard under the Analytics Section. You will see : +> +* The dashboard consists of the following segments : +1. Daily active users +1. Users in last 30 mins +1. Daily user engagement +1. Key events +1. Stability statistics +1. Audience statistics +1. User retention stats +* We can filter out the results that is displayed on the dashboard by using the filter option on the top left. We can filter the values using user properties like device model, OS, app-version, etc. +* We can also view these things under their own specific dashboards. For example the event dashboard looks like: +> +* This dashboard consists of all the predefined events along with the custom logged events as you can see in the image above. We can see detailed stats related to an event by clicking on its event name. + +### DebugView +* DebugView enables you to see the raw event data logged by your app on development devices in near real-time. +> +* _To run this, you must have ADB set up in your system._ +* To enable DebugView, write `adb shell setprop debug.firebase.analytics.app org.oppia.android` in your command line while the app runs on your emulator/device. +* Now as you play around with the app you'll see various events being logged to the console. To get more details about an event, just click on it and you'll see something like this : +> +* In the above case, we can see that the logged event : `OPEN_LESSONS_TAB` has custom parameters like `priority`, `timestamp` and `topicId` along with predefined Firebase event parameters. +* To get more specific details about the logged event, you can click on its parameters and see their values as well. +* To disable DebugView, write `adb shell setprop debug.firebase.analytics.app .none.` in your command line. + +### Funnels +* They are used to visualize and optimize the completion rate of a series of steps (events) in the app. +> +* You can filter out the results (from the top) on the basis of user properties like app-version, device model, OS-version, etc. +* To set up funnels, follow these steps : +1. In Analytics, navigate to your app. +1. Click Funnels. +1. Click NEW FUNNEL. +1. Enter a name and description for the funnel. +1. Select the first two events you want to use as steps in the funnel. +1. Click ADD ANOTHER EVENT for each additional step, and select an event. +1. Click CREATE. +> + +### Adding Custom Parameters +* Go to the Events section of Analytics and pick an event you want to add custom parameters for. +* Move the cursor to the right hand side of that event and click on the three dots and then select 'Edit Parameter Reporting'. +* You will be shown a dialog box like this : +> +* Click ADD on any of the parameters that you want to add --> select if you want it to be a text based or a number based parameter and click SAVE. Voila! You just added a custom parameter. +* It may be noted that for custom events, we can provide custom parameters via code and then integrate them here. + +### Conversions +* In Analytics, the most important events are known as Conversions. +* We can mark any of the event as a conversion event just by going to the Events -> toggling on the conversion button on the right side of the event that you want to mark as conversion. +* A summary of conversion events are shown on the main Analytics Dashboard, while other details are available in the Conversions sub-section. + +## Your own Firebase Project + +### Setting it up +* Go to the [Firebase Console](https://console.firebase.google.com) -> Log in to your gmail account -> Click on "Add Project" +* Enter your project name -> Enable/Disabe Google Analytics and move forward. +* If you enabled Google Analytics then accept the terms and select a project location and click create project. +* If you disabled Google Analytics then simply click on create project. +* Click continue on completion of the progress bar and the Project Console (like the one shown above) will open. + +### App Registration +* Once your Project Console opens up, click on 'Add an App' from the centre. This will open up : +> +* Add the required info and click on Register App. +* Then in the next step you would be required to add a config file to your repo. In our case we have to add it under our app module. +* Then click next, add the said plugins (if not already added). +* In the next step build & run the app so that the Firebase servers sync to it and click on Go to Console. +* You can check the details of your project and the apps registered to it by clicking on the settings button (present on the top of the nav bar) and then selecting 'Project settings' from it. A new page will open up with all the necessary details. + +## View events in the Android Studio debug log + +You can enable verbose logging to monitor logging of events by the SDK to help verify that events are being logged properly. This includes both automatically and manually logged events. + +You can enable verbose logging with a series of adb commands: +* `adb shell setprop log.tag.FA VERBOSE` +* `adb shell setprop log.tag.FA-SVC VERBOSE` +* `adb logcat -v time -s FA FA-SVC` + +This command displays your events in the Android Studio logcat, helping you immediately verify that events are being sent. \ No newline at end of file diff --git a/wiki/Fork-and-Clone-Oppia-Android.md b/wiki/Fork-and-Clone-Oppia-Android.md new file mode 100644 index 00000000000..dc0d9237e35 --- /dev/null +++ b/wiki/Fork-and-Clone-Oppia-Android.md @@ -0,0 +1,41 @@ + +_For a detailed explanation of the fork-and-clone process, please see this [GitHub help page](https://help.github.com/en/github/getting-started-with-github/fork-a-repo#platform-linux)._ + +## Fork and clone the Oppia Android repository + +To make code changes, please follow the following instructions +- [Using the terminal](https://gist.github.com/MaskedCarrot/ea0933311b95a108f99e3c6c106ea101#using-the-terminal) +- [Using android studio's UI based github workflow](https://gist.github.com/MaskedCarrot/ea0933311b95a108f99e3c6c106ea101#using-android-studios-ui-based-github-workflow) + +### Using the terminal + +1. Make sure that you are in the `opensource/` folder on your local machine. + +2. Click on the "Fork" button on the top-right corner (at the same level as the oppia/oppia-android repository name): +![Oppia-Android Fork](https://user-images.githubusercontent.com/9396084/71397560-eed7b600-2643-11ea-854c-ea1d57df497d.png) + +3. You can now see Oppia-Android under [your repositories](https://github.com/). It will be marked as forked from `oppia/oppia-android` +![Oppia-Android Origin Repo](https://user-images.githubusercontent.com/9396084/71338640-b0c09080-2576-11ea-8dc3-3d4a0ef59877.png) + +4. Clone this repository to your local computer by running `git clone https://github.com/USERNAME/oppia-android.git` in a terminal. + +5. To keep your local repository, forked repository and main oppia-android repository in sync, configure your remote repositories by running the following two commands in a terminal: + - `git remote -v` (this lists your current remote repositories) + - `git remote add upstream https://github.com/oppia/oppia-android` (this adds oppia/oppia-android as an upstream repo) + +### Using android studio's UI based GitHub workflow +1. Navigate to your fork, e.g. ``https://github.com//oppia-android``. +Click on **Clone or download** and copy the link (the URL will look different since you should be using your fork, **not** https://github.com/oppia/oppia-android). + +1 + +2. Now, go to **Android Studio**>**File**>**New**>**Project from Version Control**>**Git** + +2 + +3. Log in GitHub with your credentials. + +4. Paste the **URL** and click on **Clone** button. +Wait for a few minutes until Gradle build completes. + +4 \ No newline at end of file diff --git a/wiki/Frequent-Errors-and-Solutions.md b/wiki/Frequent-Errors-and-Solutions.md new file mode 100644 index 00000000000..dac1bd3240c --- /dev/null +++ b/wiki/Frequent-Errors-and-Solutions.md @@ -0,0 +1,49 @@ +## Dagger Unresolved reference +**Error**: Unresolved reference `DaggerXXTest_TestApplicationComponent` + +**Solution**: Don't worry this is not an error. Just run the test file and it will solve the error. For running tests, you can see [Oppia Android Testing](https://github.com/oppia/oppia-android/wiki/Oppia-Android-Testing) document. + +## Crashing layout tags in tablet +**Error**: java.lang.IllegalArgumentException: The tag for topic_lessons_title is invalid. Received: layout-sw600dp-port/topic_lessons_title_0 +**Solutions**: This error occurs when we remove any xml file which is related to tablet devices +To solve this +1. Uninstall the app from tablet +2. Rebuild the app. +3. Run the app again. + +## Push failed +**Error**: Failed to push some refs to `git@github.com:/oppia-android.git` + +**Solution**: If you are using Android Studio or another UI-based git to push and got the above error, this might not provide many details. Try to push using the command line. Also, we suggest using the command line, as all our scripts, project building, APK building are done using the command line as we are moving towards the Bazel build system and removing the Gradle build system. + + + +If there is an error with any of the lint failures it will be detailed in the command line itself. + +## Facing error while debugging code +Whenever you are debugging a problem, you may find it useful to keep a record of your debugging process. We often do this already in issues. Issues usually begin with a detailed description of the problem, which is followed by discussion, reports of attempted debugging steps, and what root cause was identified. However, issues' linear comment structure makes them more amenable to communication among team members than organizing your thoughts. Debugging docs, on the other hand, serve primarily to organize your thoughts. + +### Benefits +Primarily, debugging docs help keep your thoughts organized. When you have written down the steps you've already tried and the results of your investigations, you don't have to worry about forgetting your earlier work. Further, when you document your work, you force yourself to reflect on what you've already tried. Debugging docs also make it easy for you to bring someone else up to speed on your bug. + +### How to Write a Debugging Doc + +#### 1. Begin with Describing What Goes Wrong +Your description should include: + - Context: What branch or PR is the bug happening on? If this is happening locally, what system are you running, and what code changes have you made? + - How the Bug Manifests: This might include error logs that you pasted into the document, screenshots, or links to a test run on a PR. If you provide a link, copy the relevant part of the page you are linking to. This keeps the document self-contained so we can search them. It also makes the doc easier for people to read. + +#### 2. Describe Your Investigations +What did you try, and what happened when you tried it? You want to include enough detail so that someone could reproduce your investigation to see if they get the same results. + +#### 3. Document Your Guesses and Testing +After some investigation, you might have some ideas for what might be going wrong. Document your guesses and describe how you go about testing them. Report the results of that testing and describe whether you think your guess was right. What's your reasoning? + +#### 4. Continue/Review from Mentor +Keep going! Continue documenting your investigations, guesses, and tests of those guesses. You can share your debugging doc with your assigned onboarding mentor to review and help you in finding the root cause of the issue or the solution. + +#### 5. Document Your Solution +Once you figure out what the problem was, write that down too! Include an analysis of how the root cause you identify caused the errors you described at the top. Often, this will take the form of one of your suspected solutions. + +#### Get Started +Ready to get started with your own debugging doc? You can make a copy of [this template](https://docs.google.com/document/d/1OBAio60bchrNCpIrPBY2ResjeR11ekcN0w5kNJ0DHw8/edit?usp=sharing) to get started. \ No newline at end of file diff --git a/wiki/Get-Help.md b/wiki/Get-Help.md new file mode 100644 index 00000000000..a40f9c589bb --- /dev/null +++ b/wiki/Get-Help.md @@ -0,0 +1,158 @@ +## Table of Contents + +* [Communication channels](#communication-channels) + * [Email](#email) + * [GitHub Discussions](#github-discussions) + * [Google Chat or Hangouts](#google-chat-or-hangouts) + * [GitHub](#github) +* [How to Ask Good Questions](#how-to-ask-good-questions) + * [Setup-related questions](#setup-related-questions) + * [Before you ask a setup question](#before-you-ask-a-setup-question) + * [How to ask a setup question](#how-to-ask-a-setup-question) +* [General questions](#general-questions) + * [Before you ask a general question](#before-you-ask-a-general-question) + * [How to ask a general question](#how-to-ask-a-general-question) + * [Important points](#important-points) + +Here we document Oppia's main communication channels and how to ask good questions. + +## Communication channels + +If you need help, there are a few communication channels you can use. Developers usually respond within 24 hours so long as you use a channel they actually check. + +### Email + +We have several mailing lists in the form of Google Groups that you can join: + +* [oppia-announce](https://groups.google.com/forum/#!forum/oppia-announce) is for announcements of new releases or blog posts. It's not for asking questions though. +* [oppia-android-dev](https://groups.google.com/g/oppia-android-dev) is the main mailing list for communication between developers and for technical questions. You can post to it even if you're not a member of the group. This is where you can ask questions, solicit feedback, or make developer-specific announcements (e.g. a temporary GitHub outage). + +You can also email your mentor with any questions. If you don't have a mentor, complete the steps on the [wiki page for contributing code to Oppia](https://github.com/oppia/oppia-android/wiki/Contributing-to-Oppia-android) and you'll be assigned one. + +### GitHub Discussions + +If you have questions regarding Oppia Android, you can create a discussion on [GitHub Discussions](https://github.com/oppia/oppia-android/discussions/) where other developers will assist you in resolving the issue. Oppia's Welfare team monitors this chat to help out new contributors, so it's the best place to ask questions about Oppia or getting started. You can create a discussion in any of the following categories based on the question: + +* Developer announcements: All announcements that affect developers for the Oppia repository will be announced here. +* Q&A: Any questions that developers have (related to setup, navigating the codebase, or anything else). +* General Discussion: General forum for any topics that aren't Q&A. + +Feel free to drop in and [say hi](https://github.com/oppia/oppia-android/discussions/4788)! You can also refer to [this guide](https://docs.github.com/en/discussions/quickstart#creating-a-new-discussion) on how to create a new discussion. + +### Google Chat or Hangouts + +Most Oppia developers and teams use Google Chat as their primary means of communication, and they usually respond quickly. However, invites often get lost in spam folders, and some developers use a non-public email address for Google Chat. You can ask your mentor to put you in touch with a developer if you don't know their address or if they haven't acted on your invite. + +### GitHub + +If you have a question about a pull request or issue, you can also reach out to developers by at-mentioning them (e.g. `@developer-username`) in a comment and assigning them to the issue. Be sure you both at-mention and assign them! Some developers only look at their GitHub notifications (which at-mentions trigger), while others only look at what they're assigned to. + +You can even mention whole teams of people! For example, if you find an issue that is destabilizing the project, you could notify all the core maintainers by including `@oppia/core-maintainers` in your issue. The teams are all listed [here](https://github.com/orgs/oppia/teams). + +## How to Ask Good Questions + +At Oppia we don’t care how silly your question is! Just ensure your question is clear, and provide us with enough information to help us resolve it faster. We've divided the questions into 2 categories - Setup-related and General questions. You can start following the sections below to understand how you can ask each of them. + +### Setup-related questions + +#### Before you ask a setup question + +1. You can setup/install oppia-android by visiting [this page](https://github.com/oppia/oppia-android/wiki/Contributing-to-Oppia-android#install-oppia-android). Make sure you follow **all** the mentioned instructions from the beginning in a step-by-step manner. + +2. Make sure each step succeeds (verify it with the expected behavior if mentioned in the wiki). + +3. In case of any unexpected behavior/errors at any step, make sure you check out our wiki on [troubleshoot installation](https://github.com/oppia/oppia-android/wiki/Troubleshooting-Installation). + +If you are still not able to fix your error, start following the section below to raise your question on [GitHub Discussions](https://github.com/oppia/oppia-android/discussions/categories/setup-issues). + +#### How to ask a setup question + +**Note**: If you are stuck at Step X, we will assume all previous steps through X-1 were successful for you. In case there were any previously failed steps, kindly mention those too with their error logs. + +Please follow the template given below (mark x inside checkboxes to tick them) for creating a GitHub Discussions. + +```md +**Checklist** +- [ ] I have followed the [Before you ask a setup question](https://github.com/oppia/oppia-android/wiki/Get-help#before-you-ask-a-setup-question) section of the wiki. + +**System Information** +- OS: (Be specific Ex: Ubuntu 20.04 or Ubuntu 20.04 VM on MacOS 11.2.1) + +**Steps followed** + +// If you encountered this error while following a wiki page, provide a link to the page and specify which step failed. Otherwise, list what steps caused the error. These should be detailed enough for someone else to follow them. + +**Error log** + +//paste error log here + +or + +paste a screenshot + +**Approaches already used to resolve the issue** + +(eg: Link to a Stack Overflow answer or any solution that you have tried) +- enter any additional description +``` + +## General questions + +### Before you ask a general question + +* We expect that you have already **set up Oppia on your machine**, and it is successfully running. (If not, kindly do that first!) +* Prepare a debugging doc following [this template](https://docs.google.com/document/d/1OBAio60bchrNCpIrPBY2ResjeR11ekcN0w5kNJ0DHw8). For more information on how to use debugging docs, see [this link](https://github.com/oppia/oppia/wiki/Debugging-Docs). +* If there are **checks** on your PR, and you haven’t done any changes in that direction, kindly understand that sometimes they just fail due to flakiness. You should request for a re-run of those only when it’s preventing your PR from getting merged. + +### How to ask a general question + +Follow the template below for asking questions (fill in the values inside {{}} brackets and mark x inside checkboxes to tick them) to leave a comment on a pull request. Adapt the template as needed if you are using another channel. + +```md +@{{PR reviewer or Mentor username}} PTAL! + +**Checklist** +- [ ] I have filled the [CLA](https://goo.gl/forms/AttNH80OV0) and the [Oppia Contributor Survey](https://goo.gl/forms/otv30JV3Ihv0dT3C3) +- [ ] I have followed to install oppia-android mentioned [here](https://github.com/oppia/oppia-android/wiki/Contributing-to-Oppia-android#install-oppia-android) +- [ ] I have worked through the [Before you ask a setup question](https://github.com/oppia/oppia/wiki/Get-help#before-you-ask-a-setup-question) section of the wiki. + +**System Information** +- OS: (Be specific Ex: Ubuntu 20.04) +- [ ] Virtual machine + +**About the issue** +- {{Describe the problem}} (eg. Failing robolectric tests: "The test has been + failing repeatedly since {{x}} previous runs, and I haven’t done any + changes related to it. Requesting a re-run.") + +**Approaches already used to resolve the issue** + +(eg: Link to a Stack Overflow answer or any solution that you have tried) +- #{{Link to the debugging doc}} +- enter any additional description +``` + +### Important points + +* If you are unable to push changes due to some reason, you can create a [patch file](https://docs.gitlab.com/omnibus/development/creating-patches.html) and share it with the person you're asking for help. + +* If you are facing issues in completing the assigned task, you can create a PR on your fork of the oppia-android repository, troubleshoot your problem on that pull request with help from your mentor, and then create a new PR on the original oppia-android repository. + +* If you have not made a PR yet, because you are not sure: + + * what the issue is about, or + * which files have to be modified, or + * if your approach towards the solution is correct + + Then ask for help by commenting with your doubt/suggested approach on the issue page itself. If you don’t get any response within **24 hours**, please leave a note on [GitHub Discussions](https://github.com/oppia/oppia-android/discussions/categories/q-a-contacting-folks) so that the developer workflow team is aware and can help you. + +* If you want to have a discussion on your approach, but aren’t ready to make a PR yet, you can create a [public gist](https://docs.github.com/en/get-started/writing-on-github/editing-and-sharing-content-with-gists/creating-gists) and include the link to it in your question. It’s always better to see the code you are talking about! + +* **Avoid asking for help from people via emails or direct messaging.** We encourage everyone to ask for help on a common channel so that whoever sees your query first can help you or guide you how to take your query forward. Note that directing questions to your mentor is fine. + + * Comment on the issue page or the PR if your question is very specific. + + * Use [GitHub Discussions](https://github.com/oppia/oppia-android/discussions) if your question is not issue-specific. + +### Welfare Team +If you still feel blocked even after following the above steps or are unsure who to contact, you can reach out to the welfare team. You can also ping the welfare team by at-mentioning the welfare team: @oppia/android-welfare-team diff --git a/wiki/Gradle--Bazel-Migration-Best-Practices-and-FAQ.md b/wiki/Gradle--Bazel-Migration-Best-Practices-and-FAQ.md new file mode 100644 index 00000000000..98ff63f4278 --- /dev/null +++ b/wiki/Gradle--Bazel-Migration-Best-Practices-and-FAQ.md @@ -0,0 +1,61 @@ +The following document explains best practices & provides support for the Gradle to Bazel migration. For more information on setting up Bazel, read [Oppia Bazel Setup Instructions](https://github.com/oppia/oppia-android/wiki/Oppia-Bazel-Setup-Instructions). + +This is a living document that will grow as more questions get asked and answered. + +## What are we doing? + +Currently, the libraries in a module's BUILD.bazel file contain most of their dependencies as files in srcs dependencies. We are going to break out smaller pieces of these source files into their own libraries. Eventually, the module libraries will depend on a smaller set of these libraries as its direct dependencies. + +In the following example, we have a set of dependencies that are specific to a KotlinFile.kt file. We have an ExampleLibrary that has both the Kotlin file as srcs dependency and the underlying deps dependencies as its own direct deps dependencies. + +![Before diagram](https://user-images.githubusercontent.com/12983742/108904850-deb6c000-75d3-11eb-9156-c01ea8e8e471.png) + +When we create a new library, we can move this set of dependencies to the new library. + +![After diagram](https://user-images.githubusercontent.com/12983742/108904926-f42bea00-75d3-11eb-88f0-37247d7f284b.png) + +This is not always possible, since there might be other files relying on the same dependencies so in our transition phase we can end up with something like this: + +![Alternative after diagram](https://user-images.githubusercontent.com/12983742/108904987-0148d900-75d4-11eb-81f9-0887b95749ce.png) + +## General tips + +- Build early and build often. This will help you catch issues and Bazel can also suggest dependencies to add to your library in some cases. +- Look at other BUILD.bazel files in the codebase for an idea on how to lay out packages. +- Migrating files will always involve introducing new libraries, and then hooking those up to the top-level module libraries. There are ways of making this a bit easier: + - While performing the migration, focus on making sure the new library builds first, e.g.: ``bazel build ///src/main/java/org/oppia/app//:all``. + - After the new libraries contain all of the files they need to, their dependencies are correct, and they build, make sure the app also builds by running: ``bazel build //:oppia``. + + +## FAQ + +### I've finished writing a BUILD.bazel file containing new libraries. What now? + +1. Remove all source files from the module's BUILD.bazel file that are now contained in the new BUILD.bazel file +2. Add the new BUILD.bazel file libraries as dependencies for the module's top-level library (these dependencies should be changed in the same library whose source files were updated in #1). +3. Try to remove the dependencies that the new BUILD.bazel file's libraries depend on from the old libraries whose source files were changed (note that these may still be needed by existing source files in those libraries). This can be done by removing each dependency one-at-a-time and trying to rebuild the library to see if the library builds. + +### I'm getting a Bazel error: "Error while obtaining the sha256 checksum of." + +If you are getting something similar to this: +``` + Traceback (most recent call last): + File "/home/titan/.cache/bazel/_bazel_titan/d2121258671c00ac2cf78dbca73dac8b/external/rules_jvm_external/coursier.bzl", line 467, column 17, in _coursier_fetch_impl + fail("Error while obtaining the sha256 checksum of " +Error in fail: Error while obtaining the sha256 checksum of v1/https/maven.google.com/android/arch/core/common/1.1.1/common-1.1.1.jar: src/main/tools/process-wrapper-legacy.cc:80: "execvp(python, ...)": No such file or directory +``` +From the error trace, we see that the tool can't find python. Make sure that you have python in your PATH. + +### When installing the app, I see INSTALL_FAILED_VERSION_DOWNGRADE or INSTALL_FAILED_UPDATE_INCOMPATIBLE + +In the event that, when trying to install a Bazel-built version of the app, you see one of the following errors: +- ``adb: failed to install bazel-bin/oppia.apk: Failure [INSTALL_FAILED_VERSION_DOWNGRADE]`` +- ``adb: failed to install bazel-bin/oppia.apk: Failure [INSTALL_FAILED_UPDATE_INCOMPATIBLE]`` + +You will need to uninstall the existing version of the Oppia Android app and then try the installation again. You can either uninstall through the system's package menu UI, or run the following command: + +``` +adb shell pm uninstall org.oppia.android +``` + +(If the command above works correctly, you will see 'Success' in the output). \ No newline at end of file diff --git a/wiki/Guidance-on-submitting-a-PR.md b/wiki/Guidance-on-submitting-a-PR.md new file mode 100644 index 00000000000..40761d5b8aa --- /dev/null +++ b/wiki/Guidance-on-submitting-a-PR.md @@ -0,0 +1,212 @@ +**Working on your first pull request?** Pull requests (PRs) can be tricky to understand at first, so if the instructions on this page don't make sense to you, check out these resources: +- The free series [How to Contribute to an Open Source Project on GitHub](https://app.egghead.io/series/how-to-contribute-to-an-open-source-project-on-github) +- [Atlassian's tutorial on pull requests](https://www.atlassian.com/git/tutorials/making-a-pull-request). + +Here are the steps for making a PR to the Oppia Android codebase: +1. [Make a local code change](#step-1-making-a-local-code-change) +2. [Create a PR on GitHub](#step-2-create-a-pr-on-github) +3. [Address review comments until all reviewers give LGTM](#step-3-address-review-comments-until-all-reviewers-give-lgtm) (this section is important, please read it carefully) +4. [Tidy up and celebrate!](#step-4-tidy-up-and-celebrate-confetti_ball) + +Note: If your change involves more than around 500 lines of code, we recommend first creating a [design doc](https://github.com/oppia/oppia/wiki/Writing-design-docs). This helps avoid duplication of effort, and allows us to offer advice and suggestions on the implementation approach. + +## Step 1: Making a local code change + +Before you make a PR, you'll need to make and test the changes locally. To do this, please follow the following instructions carefully! Otherwise, your code review may be delayed. + - [Making a local code change using the terminal](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#making-a-local-code-change-using-the-terminal) + - [Making a local code change using Android Studio's UI-based Github workflow](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#making-a-local-code-change-using-android-studios-ui-based-github-workflow) + +### Making a local code change using the terminal + +1. **Update your repository.** The new branch should be based on the latest code in develop, so checkout the latest version of develop like this: + + ``` + git fetch upstream + git checkout develop + git merge upstream/develop + ``` + +2. **Create a new branch with a descriptive name.** In the example below, the branch is named `your-branch-name`. The branch name should be lowercase and hyphen-separated, e.g. `fuzzy-rules`. Make sure that your branch name doesn't start with `develop`, `release` or `test.` + + ``` + git checkout -b your-branch-name + ``` + +3. **Make commits locally to your feature branch.** Each commit should be self-contained and have a descriptive commit message that helps other developers understand why the changes were made. However, do not write "Fix #ISSUE_NUMBER" (e.g. "Fix #99999") in your commit messages (or any of the other closing phrases from GitHub's documentation), as this will cause Github to close the original issue automatically. + + You can change your most recent commit message using `git commit --amend`. *Note: It is difficult to change any commit messages other than your most recent one or messages on commits that have been pushed, so write your commit messages carefully!* + + Before making the commit, do some sanity-checks: + - Ensure that your code follows the style rules. + - Run robolectric and espresso tests to make sure you haven't broken anything! If you would to know more about testing on oppia-android, check out [Oppia Android Testing](https://github.com/oppia/oppia-android/wiki/Oppia-Android-Testing) + - Run git status to check that your changes are what you expect. To inspect the changes made to any particular file, use git diff your-file-name. + + Stage all your changes (the `.` at the end refers to your current directory): + + ``` + git add . + ``` + + To actually make the commit, run: + + ``` + git commit -m "{{YOUR COMMIT MESSAGE HERE}}" + ``` + + *Note: There is no maximum or minimum number of commits required in a PR. Instead of aiming for a certain number, you should try to make each commit a logical "chunk" of work. There are many opinions about how big commits should be, but a useful rule of thumb is that you should be able to read the first lines of all your commit messages to get a good idea of what you changed. If you find yourself needing lots of clauses to capture what you changed, your commit is probably too big.* + +4. **Push changes to your GitHub fork.** Before pushing, make sure to check the following things, otherwise you will incur delays with the review process or the automated checks: + - Do some manual testing on your local instance of Oppia to check that you haven't broken anything. This is important to avoid breakages. + - Use a tool like `git diff upstream/develop` to check that the changes you've made are exactly what you want them to be, and that you haven't left in anything spurious like debugging code. + + We don't allow force-pushing at Oppia, so once you push your commits, you can't change them. You can still add new commits though. + + When you're ready to push, run: + ``` + git push origin {{YOUR BRANCH NAME}} + ``` + + Make sure to do this from the command line (and not GitHub's Desktop client), since this also runs some important presubmit checks before your code gets uploaded to GitHub. If any of these checks fail, read the failure messages and fix the issues by making a new commit (see step 3), then repeat the previous instructions to retry the push. Do not bypass these presubmit checks! The checks get run on your PR too, so if you bypass failures on your local machine, you'll just have to fix them later when they fail on the PR. + +### Making a local code change using Android Studio's UI-based Github workflow + +1. **Update your repository.** The new branch should be based on the latest code in develop, so checkout the latest version of develop, like this: + + On the bottom right side of your android studio screen you will find your branch name. Click on **branch_name** > **develop** > **Update**: + + 10 + +2. **Create a new branch with a descriptive name.** On the bottom right side of your Android Studio screen, you will find your branch name. Click on **branch_name** > **develop** > **New Branch from Selected** > _Enter your new branch name_ > **OK**. + + Then, create a new branch. In this example the branch is named `github`. The branch name should be lowercase and hyphen-separated, e.g. `fuzzy-rules`. Make sure that your branch name doesn’t start with `develop`, `release` or `test.` + + 12 + + If you want to go back to "develop" or any other branch/"check-out any branch" you may right-click and find options for that: + + 11 + +3. **Make commits locally to your feature branch.** Now, when you create a new activity, it will be unversioned and therefore displayed with a reddish-brown colour file name. To add the files to Git/version Select files and click the "Add" button. + + 5 + + New/modified files will be displayed in green /blue colour respectively in the project window. (select file/folder/directory Ctrl+Alt|+A to manually version an unversioned file/files) + + 8 + + Click on the green tick symbol to commit your files. + + 9 + + Now click on commit(This will save the reference of your changes for pushing to Git). If there are errors or warnings, review the code change and fix them before committing. + + 13 + +4. **Push changes to your GitHub fork.** We need to push the code changes to local branch (origin) and upstream (remote). Right click **app > Git > Repository > Push**: + + 15 + + Your new branch needs to be added to both origin and upstream remote. Once you push a branch to Git, you won't be able to rename it, so be sure about the naming before pushing: + + 16 + + +## Step 2: Create a PR on GitHub + +Once your feature is ready, you can open a pull request (PR)! Here is how to do so: + +- Go to your fork on GitHub, select your branch from the dropdown menu, and click "pull request". Ensure that the base repository is oppia/oppia-android and that the base branch is develop. The head repository should be your fork, and the head branch should be your branch. If you don't see the repository, click the link to compare across forks. +- On this page, you can also see your commits and your changes. Read these changes carefully to make sure that the changes are correct. This is a good way to catch obvious errors that would otherwise lead to delays in the review process. +- Click "Create pull request". + +You have successfully created a pull request! Now, wait for your code to get reviewed! While you're waiting, it's totally fine to start work on a new PR if you like. Just follow these instructions again from the beginning. + +**Note:** After a while, check your pull request to see whether the CI checks have passed. If not, follow our [instructions](https://github.com/oppia/oppia-android/wiki/Interpreting-CI-Results) to diagnose PR failures. + +### Important Points to Keep in Mind + +1. **UI related issue/bug fix**: If your PR introduces changes to the UI/UX part of the app, do the following: + - Include "before" and "after" screenshots (and possibly a video if needed). + - Test the UI/UX with [Accessibility Scanner](https://support.google.com/accessibility/android/answer/6376570?hl=en). _(Tip: All your dimensions should be in multiples of 4dp.)_ +2. **Bug fixes**: While fixing an issue labelled **Bug**, make sure to write test cases which actually catch that bug. +3. **Self Review**: Always self-review your PR first before assigning it to anyone else, so that you can fix nit changes at very early stage. This makes the review process faster. +4. **Undo unnecessary changes**: Sometimes, Android Studio automatically makes changes to files based on your local studio configuration. Mostly these changes are from `.idea` folder. You can revert these unnecessary changes by following these steps: + - Once your PR is created, go to the `Files changed` section available on top of your pull request. For example: + + - Then, check all the files in this section and if you see any change which was not done by you, revert it locally and commit again to the pull request. The `Files changed` section should contain only those changes which were done by you. + + +### Clarification regarding **Assignees** and **Reviewers** section. +1. **Reviewers**: This section is generally ignored by anyone who looks at the PR. It will be filled automatically by Oppiabot. Once this section is filled out, it generally should not change throughout the timeline of the PR. +2. **Assignees**: The main section to look at is the 'Assignees' field, which indicates the person(s) whom the PR is currently blocked on. Specifically: + - Initially, when the PR is submitted, the **Assignees** and **Reviewers** sections should be the same. + - Once a reviewer has reviewed the PR, they should de-assign themselves and assign it back to the PR author. + - Similarly, once the author has made the requested changes, they should assign it back to the appropriate reviewer and de-assign themselves. + + + +## Step 3: Address review comments until all reviewers give LGTM + +When your reviewer has reviewed the code, you'll get an email. You'll need to respond both to the comments and within the code: + +1. Make a new commit addressing the comments you agree with, and push it to the same branch. (Continue to use descriptive commit messages. If your commit addresses lots of disparate review comments, it's fine to refer to the original commit message and add something like "(address review comments)".) + + - **Always make commits locally, and then push to GitHub.** Don't make changes using the online GitHub editor -- this bypasses lint/presubmit checks, and will cause the code on GitHub to diverge from the code on your machine. + + - **Never force-push changes to GitHub, especially after reviews have started.** This is disallowed and may result in your PR being closed, because it overwrites history on GitHub and makes the incremental changes harder to review. + + - If you encounter merge conflicts, make sure to resolve them using [the terminal](#Resolving-merge-conflicts-using-the-terminal) or [Android Studio](#Resolving-merge-conflicts-using-Android-Studio). (Click the links for more details on how to do this.) + +2. **Reply to each comment** via the Files Changed tab, choosing the “Start a review” option for the first comment. Each reply should be either “Done” or a response explaining why the corresponding suggestion wasn’t implemented. + +3. When you’ve responded to all comments, submit the review to add all your messages to the main thread. **All comments must be responded to** and fully addressed before LGTM can be given. You can use the green button in the top right of the "Files Changed" tab to submit any pending comments. + + - If you would like the reviewer to take another look, (a) write a top-level comment** explicitly asking the reviewer to take another look (e.g. "@XXX PTAL"), and (b) set them as the assignee for the PR. + +At the end, the reviewer will merge the pull request. Note however PRs will only be merged if there are no more requested changes, and all conversation threads have been brought to a conclusion. + +### Tips for getting your PR submitted +1. Keep track of the **Assignees** section, and make sure it's always up to date -- it should always reflect the "next person(s)" who have actionable items left on the PR. If you're ever the assignee, do whatever is currently needed on your end so that you can take yourself off the list (usually, this means addressing open review comments). +2. Make sure to **address all comments** before sending a PR back to a reviewer. Each comment thread should have a reply from you before you send the PR back. This can be either "Done", a follow-up clarifying question, or an explanation for why you don't think a suggestion should be implemented. Don't forget to actually submit your replies (using the green button at the top right of the "Files Changed" tab), otherwise reviewers won't be able to see them! +3. Do not "resolve" a thread conversation if you didn't open it. Just re-assign the PR to the reviewer once you address all of the comments. This helps with keeping track of which comments are still "in play". +4. Once the PR is ready to merge, add a top-level comment confirming the merge decision, and merge the PR. If any issues need to be filed subsequently, file them and refer to them in the PR via a comment. + + +### Appendix: Resolving merge conflicts using the terminal + +To resolve conflicts between ‘new-branch-name’ (in your fork) and ‘develop’ (in the oppia repository), run: + +``` +git checkout new-branch-name +git fetch upstream +git merge upstream/develop +git commit -a +git push origin new-branch-name + ``` + +### Appendix: Resolving merge conflicts using Android Studio + +Usually Git is able to automatically merge files when pulling changes from another branch. But sometimes a conflict arises when two separate branches have made edits to the same line in a file, or when a file has been deleted in one branch but edited in the other. + +You can use Android Studio to resolve merge conflicts through its UI-based Git features. Here's how to do it: + +- Go to VCS > Git > Pull (set the remote to be upstream and branch to be develop). Or, use VCS > Git > Resolve Conflicts if you have already pulled the changes but haven’t resolved the conflicts. + + !["Pull Changes" dialog box in Android Studio](https://user-images.githubusercontent.com/10575562/154797367-bf8cbefd-c6da-4a82-8210-806d70411b05.png) + +- Usually, this will automatically merge the files. However, in the case of conflicts, it will prompt you to tell it what to do with those files. + + !["Conflicts" dialog box in Android Studio](https://user-images.githubusercontent.com/10575562/154797370-2d4d7d6c-42c4-4581-a83e-fcce727d5d70.png) + +- You can either directly accept the changes from develop (discarding your local changes) or keep your own changes (discarding the changes from develop) based on the situation but it is suggested to go through each file line by line using the merge button in the prompt. + + !["Conflicts diff" window in Android Studio](https://user-images.githubusercontent.com/10575562/154797371-9e2596ab-ad0c-4fa3-93ca-6e874528bb99.png) + + The file on the left shows changes from your local working branch and the file on the right shows the changes from the develop branch while the centre file being the final state of it. You can decide accordingly which change (or both) you want to keep for each conflict one by one using the arrows and cross sign on those highlighted lines. Once the conflicts are successfully resolved you can then commit and push your changes to your working branch. + + +## Step 4: Tidy up and celebrate! :confetti_ball: +After the PR status has changed to "Merged", delete the feature branch from both your local clone and the GitHub repository. Congratulations, you have contributed to the Oppia Android project! + +If you have already completed 2 pull requests and been added as a collaborator to the project, you should also add a changelog label. If you are a new contributor, you don't have permission to do this. Don't worry! Oppiabot will automatically ask someone to do it for you. + diff --git a/wiki/Home.md b/wiki/Home.md new file mode 100644 index 00000000000..3b915b6cc45 --- /dev/null +++ b/wiki/Home.md @@ -0,0 +1,13 @@ +Thanks for your interest in contributing to the Oppia project and making it easier for students to learn online effectively and enjoyably! + +There are many ways to help out and become an Oppia contributor, from joining our team of lesson creators to fixing a bug in the [codebase](https://github.com/oppia/oppia-android/). Don't worry if you're new to "open source" or still a student -- many of our contributors are too, and we'd be happy to provide mentorship and support if this is your first time contributing to an open-source project. The main thing is that you care deeply about helping people worldwide learn something better and that you're responsible and reliable. + +To make it easier to get started, we've catalogued some different ways to help out. Please feel free to take a look through them and see if any interest you: + + * [Coders](https://github.com/oppia/oppia-android/wiki/Contributing-to-Oppia-android) + +If you are interested in working on Oppia's web repository, you should also look at the [oppia/oppia repository](https://github.com/oppia/oppia). + +If you run into any problems, you can ask questions on [GitHub Discussions](https://github.com/oppia/oppia-android/discussions). You can also check out the [developer mailing list](https://groups.google.com/forum/?fromgroups#!forum/oppia-android-dev). + +Finally, if, after reading the above links, you'd like to help but aren't sure how -- don't worry! The Oppia project is very multi-faceted, and we'd be glad to help you find something to do that matches your interests and skills. Just fill out [this form](https://forms.gle/jEytndtgdsx7BrnV6) to let us know what you'd like to help with, or write to us at [volunteer@oppia.org](mailto:volunteer@oppia.org) and tell us a bit about yourself and what you'd like to do. We'll do our best to help you get started! diff --git a/wiki/Installing-Oppia-Android.md b/wiki/Installing-Oppia-Android.md new file mode 100644 index 00000000000..5fb0aea95d9 --- /dev/null +++ b/wiki/Installing-Oppia-Android.md @@ -0,0 +1,68 @@ +This wiki page explains how to install Oppia Android on your local machine. If you run into any issues with the installation process, please feel free to ask on [GitHub Discussions](https://github.com/oppia/oppia-android/discussions/categories/q-a-installation), so that we can help you while also making these instructions better for other developers. Thanks! + +## Table of Contents + +- [Prepare developer environment](#prepare-developer-environment) +- [Install oppia-android](#install-oppia-android) +- [Run the app from Android Studio](#run-the-app-from-android-studio) + + +## Prepare developer environment + +1. Download/Install the latest version of [Android Studio](https://developer.android.com/studio/?gclid=EAIaIQobChMI8fX3n5Lb6AIVmH8rCh24JQsxEAAYASAAEgL4L_D_BwE&gclsrc=aw.ds#downloads). + +2. Download and Install **Java 8** using the links from [the Java website](https://www.java.com/en/download/). + - **Note for Windows users:** Make sure to also set up the PATH system variable correctly for `Java`, following [these instructions](https://www.java.com/en/download/help/path.html). + - [Instructions](https://www.java.com/en/download/help/linux_install.html) for Linux users. + - [Instructions](https://www.java.com/en/download/help/mac_install.html) for Mac users. + +3. In Android Studio, open Tools > SDK Manager. + - In the "SDK Platforms" tab (which is the default), select `API Level 28` and also `API Level 31` (for Bazel support). + - Also, navigate to the "SDK Tools" tab, click the "Show Package Details" checkbox at the bottom right, then click on "Android SDK Build-Tools 34-rc1" and select 29.0.2 (this is needed for Bazel support). + + Then, click "Apply" to download and install these two SDKs/Tools. + +## Install oppia-android + +Please follow these steps to set up Oppia Android on your local machine. + +1. Create a new, empty folder called `opensource/` within your home folder. Navigate to it (`cd opensource`), then [fork and clone](https://github.com/oppia/oppia-android/wiki/Fork-and-Clone-Oppia-Android) the Oppia-Android repo. This will create a new folder named `opensource/oppia-android`. Note that contributors who have write access to the repository may either create branches directly on oppia/oppia-android or use a fork. + + **Note**: Please keep the folder name as `oppia-android`. Changing the project folder name might lead to future issues with running the pre-push checks on your machine. + +2. Run the setup script, which adds some development tools for Oppia Android (ktlint, checkstyle, etc.): + + - For Mac or Linux + 1. Open a terminal and navigate to `opensource/oppia-android/`. + 2. Run the script `bash scripts/setup.sh`. + + - For Mac with Apple M1 chip + 1. Locate Terminal in Finder. + 2. Right-click and create a duplicate Terminal (and rename it accordingly, say Terminal x86, to avoid confusion). + 3. In the Terminal x86, right-click and click "Get info", and check the option "Open using Rosetta". + 4. Navigate to `opensource/oppia-android/` in Rosetta. + 5. Finally, run `bash scripts/setup.sh` in Terminal x86 and all the required files should be generated. (You should see messages like `Ktlint file downloaded`, etc.) + + - For Windows + 1. Install [Git Bash Command Line](https://gitforwindows.org/) + 2. Open Git Bash Command Line. + 3. Navigate to `opensource/oppia-android/`. + 4. Run the script `bash scripts/setup.sh`. + 5. Download the [google_checks.xml](https://github.com/checkstyle/checkstyle/blob/14005e371803bd52dff429904b354dc3e72638c0/src/main/resources/google_checks.xml) file. To do this, you can simply right-click on the `Raw` button and click on `Save Link as`. + 6. Copy this file to the directory where Git is installed (usually C:/Program Files/Git/). + +3. In Android Studio, select `File > Open`, navigate to `opensource/oppia-android/`, and click `OK` to load the project. + +4. Click the elephant icon in the toolbar ("Sync Gradle") to ensure that all the correct dependencies are downloaded. (In general, you'll want to do this step any time you update your dependencies.) + +## Run the app from Android Studio + +1. Go to Tools > Device manager, click "Create Virtual Device...". Then: + + - Select a preferred device definition. In general, any device is fine, but you can use Pixel 3a as a default (if you're developing for phones) or Nexus 7 (if you're developing for tablets). After selecting a device, click "Next" at the bottom right to continue. + - Select a system image (in general, API Level 28, unless you're an M1 Mac user, in which case use API Level 29). Then click "Next". + - Click "Finish" to complete setup. + +2. To run the app, select the emulator device you want from the dropdown menu to the left of the "Run" button in the toolbar. + +3. Finally, click the "Run" button. \ No newline at end of file diff --git a/wiki/Instructions-for-making-a-code-change.md b/wiki/Instructions-for-making-a-code-change.md new file mode 100644 index 00000000000..ca91ee920ff --- /dev/null +++ b/wiki/Instructions-for-making-a-code-change.md @@ -0,0 +1,79 @@ +**Important:** Please read the [Oppia Android coding style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide) before making any code changes. + +**Working on your first Pull Request?** You can learn how from this free series: [How to Contribute to an Open Source Project on GitHub](https://egghead.io/series/how-to-contribute-to-an-open-source-project-on-github). + +*If your change isn't trivial, please [talk to us](https://gitter.im/oppia/oppia-android) before you start working on it -- this helps avoid duplication of effort, and allows us to offer advice and suggestions. For larger changes, it may be better to first create a short doc outlining a suggested implementation plan, and send it to the Android dev team for feedback.* + +The following instructions describe how to make a one-off code change using a feature branch. (In case you're interested, we mainly use the [Gitflow workflow](https://www.atlassian.com/git/tutorials/comparing-workflows/gitflow-workflow).) Please follow them carefully, otherwise your code review may be delayed. + +You might also find this reference for the [Android Studio UI-based Github workflow](https://github.com/oppia/oppia-android/wiki/Android-Studio-UI-based-Github-workflow) helpful. + +1. **Choose a descriptive branch name.** It should be lowercase and hyphen-separated, such as `splash-screen`. Also, it shouldn't start with `hotfix` or `release`. +2. **Before coding anything, create a new branch with this name, starting from 'develop'.** I.e., run: + + ``` + git fetch upstream + git checkout develop + git merge upstream/develop + git checkout -b your-branch-name + ``` + +3. **Make commit(s) to your feature branch.** Each commit should be self-contained and have a descriptive commit message that helps other developers understand why the changes were made. However, **do not write "Fix #ISSUE_NUMBER"** (e.g. Fix #99999) in your commit messages, as this will cause Github to close the original issue automatically. You can rename your commit messages using `git commit --amend`. + + * Before making the commit, do some sanity-checks: + * Start up a local instance of Oppia and do some manual testing in order to check that you haven't broken anything! + * Ensure that your code follows the [style rules](https://github.com/oppia/oppia-android/wiki/Coding-style-guide) and that it is well-tested. + * Ensure that the code has no [lint errors](https://github.com/oppia/oppia-android/wiki/Ktlint-Guide) and passes all automated tests (which will automatically run when you create your pull request). + * Use a tool like `git diff` or `meld` to check that the changes you've made are exactly what you want them to be, and that you haven't left in anything spurious. Make sure to do this _before_ you push. + + * To actually make the commit and push it to your GitHub fork, run: + + ``` + git commit -a -m "{{YOUR COMMIT MESSAGE HERE}}" + git push origin {{YOUR BRANCH NAME}} + ``` + + Make sure to do this from the command line (and not GitHub's Desktop client), since this also runs some important presubmit checks before your code gets uploaded to GitHub. **If any of these checks fail, the push will be interrupted**. If this happens, fix the issues that the tests tell you about and **repeat the instructions above** ('commit' and then 'push'). + +4. **When your feature is ready to merge, create a pull request.** + * Go to your fork on GitHub, select your branch from the dropdown menu, and click "pull request". Ensure that the 'base' repository is the main oppia repo (not your fork) and that the 'base' branch is 'develop'. + * Add a descriptive title explaining the purpose of the PR (e.g. "Fix issue #bugnum: add a warning when the user leaves a page in the middle of an exploration."). + * If the PR resolves an issue on the issue tracker, the title must start with **"Fix #bugnum: "**. This will be the case for most PRs. + * However, if your PR fixes part of a bigger issue (e.g. the first-contributor-issues listed above), please use **"Fix part of #bugnum: "** instead. Otherwise, GitHub will close the entire issue automatically when your PR is merged. + * Fill out the PR checklist (this shows up in the body of the PR when you create it), ensuring that your PR description includes the issue number (e.g. "This PR fixes issue #bugnum" or "This PR fixes part of issue #bugnum"). + * Click "Create pull request", then **immediately** check the "Files changed" tab on your PR on GitHub and read it carefully to make sure that the changes are correct (e.g., that you haven't left out important files that should be part of the PR. (If not, please fix this by making additional commits, or by closing this PR and submitting a new one, before requesting a review.) This is a good way to catch obvious errors that would otherwise lead to delays in the review process. + * Request a review from the issue's "owner" **and** also set them as the PR assignee. + * Leave a top-level comment on your PR saying "@{{reviewer}} PTAL", where {{reviewer}} is the GitHub username of your reviewer. ("PTAL" means "Please take a look".) + * Then, wait for your code to get reviewed! While you're doing so, it's totally fine to start work on a new PR if you like. Just make sure to **checkout the develop branch** and sync to HEAD before you check out a new branch, so that each of your feature branches is based off the main trunk. + +5. #### **Address review comments until all reviewers give LGTM ('looks good to me').** + * When your reviewer has reviewed the code, you'll get an email. You'll need to respond in two ways: + * Make a new commit addressing the comments you agree with, and push it to the same branch. (Continue to use descriptive commit messages. If your commit addresses lots of disparate review comments, it's fine to refer to the original commit message and add something like "(address review comments)".) + * **Always make commits locally, and then push to GitHub.** Don't make changes using the online GitHub editor -- this bypasses lint/presubmit checks, and will cause the code on GitHub to diverge from the code on your machine. + * **Never force-push changes to GitHub, especially after reviews have started.** This will delay your review, because it overwrites history on GitHub and makes the incremental changes harder to review. + * In addition, reply to each comment via the Files Changed tab, choosing the "Start a review" option for the first comment. Each reply should be either "Done" or a response explaining why the corresponding suggestion wasn't implemented. When you've responded to all comments, submit the review to add all your messages to the main thread. All comments must be responded to and resolved before LGTM can be given. + * Resolve any merge conflicts that arise. To resolve conflicts between 'new-branch-name' (in your fork) and 'develop' (in the oppia repository), run: + + ``` + git checkout new-branch-name + git fetch upstream + git merge upstream/develop + ...[fix the conflicts -- see https://help.github.com/articles/resolving-a-merge-conflict-using-the-command-line]... + ...[make sure the tests pass before committing]... + git commit -a + git push origin new-branch-name + ``` + * Once you've finished addressing everything, and would like the reviewer to take another look: + * Start a dev server in order to make sure that everything still works. + * Check that the changes in the "Files Changed" tab are what you intend them to be. + * **Write a top-level comment** explicitly asking the reviewer to take another look (e.g. "@XXX PTAL"), and set them as the assignee for the PR. + * At the end, the reviewer will merge the pull request. + +6. **Tidy up!** After the PR status has changed to "Merged", delete the feature branch from both your local clone and the GitHub repository: + + ``` + git branch -D new-branch-name + git push origin --delete new-branch-name + ``` + +7. **Celebrate.** Congratulations, you have contributed to the Oppia Android project! diff --git a/wiki/Internationalization.md b/wiki/Internationalization.md new file mode 100644 index 00000000000..be7164ef4f4 --- /dev/null +++ b/wiki/Internationalization.md @@ -0,0 +1,39 @@ +We are grateful for the support of [Translatewiki](https://translatewiki.net/w/i.php?title=Special:Translate&group=oppia-android-app&filter=%21translated&action=translate) in contributing internationalized platform strings for the Oppia Android app! (Note that Translatewiki only provides support for translations of platform UI strings; content string translations are handled via the Contributor Dashboard on the Oppia.org website.) + +## Helping out with translations + +If you would like to help out with translations, you can do so by visiting the [Translatewiki dashboard](https://translatewiki.net/w/i.php?title=Special:Translate&group=oppia-android-app&filter=%21translated&action=translate) and picking a supported language from the dropdown menu on the right. + +## Minimal set of topics + +In addition to platform translations, we need the Oppia lessons themselves to be sufficiently translated before we can offer the app in a given language. + +Currently, we define "sufficiently translated" to mean that the **minimal set** of topics are fully translated. This minimal set covers 6 topics: Place Values, Addition and Subtraction, Multiplication, Division, Fractions and Ratios. + +## Policy for enabling new languages + +Note that we only enable a select set of languages for Translatewiki, since releasing the lessons on the Android app is gated by the lessons themselves needing to be translated (see above). Therefore, in order not to waste translators' efforts, the policy for enabling languages for translation on Translatewiki is as follows: + + - The translations for all topics in the [minimal set](https://github.com/oppia/oppia-android/wiki/Internationalization#minimal-set-of-topics) on Oppia.org should be complete, or very close to complete. + +And the policy for enabling a language in the Android app is as follows: + + - The translations for all topics in the [minimal set](https://github.com/oppia/oppia-android/wiki/Internationalization#minimal-set-of-topics) on Oppia.org should be complete. + - All oppia-android-app translations for that language are completed in Translatewiki. + +## Procedure for enabling new languages + +### To enable a new language in Translatewiki + +Please do the following steps: + + 1. Verify that translations for all lessons in the [minimal set](https://github.com/oppia/oppia-android/wiki/Internationalization#minimal-set-of-topics) on Oppia.org are complete (or very close to complete) in that language, by going to the Contributor Dashboard and checking that there are no (or very few) translations left to do. + 2. Contact Ben/Sean and ask them to add the language to the oppia-android-app project. (They will do this by contacting translatewiki@translatewiki.net .) + +### To enable a new language in the Android app + +Please do the following steps: + + 1. Verify that translations for all lessons in the [minimal set](https://github.com/oppia/oppia-android/wiki/Internationalization#minimal-set-of-topics) on Oppia.org are complete in that language, by going to the Contributor Dashboard and checking that there are no translations left to do. + 2. Verify that the translations on [Translatewiki](https://translatewiki.net/w/i.php?title=Special:Translate&group=oppia-android-app&filter=%21translated&action=translate) for the Oppia Android app are complete in that language. + 3. In consultation with Ben, make a PR adding support for that language ([here](https://github.com/oppia/oppia-android/pull/4307/files) is an example for Swahili). \ No newline at end of file diff --git a/wiki/Interpreting-CI-Results.md b/wiki/Interpreting-CI-Results.md new file mode 100644 index 00000000000..b0135d27293 --- /dev/null +++ b/wiki/Interpreting-CI-Results.md @@ -0,0 +1,16 @@ +## How to find error message for Failing CI checks + +Creating a pr or updating a pr runs all the CI checks, which can sometimes fail if the code changes have affected some other part of the app or if the code changes don’t need some reformatting and docs. In these cases understanding the error and fixing it requires how to find the error. + +![image](https://user-images.githubusercontent.com/54740946/135907913-3e542b99-ff10-420e-819c-dac818033679.png) + + +All the checks of the latest commit in the pull request are displayed at the bottom of the pr. Scroll to all the checks and open details for the failing checks which display logs of each check. + +Each check contains multiple jobs and now select the job with the failure. +Example in the below check the second job has some error or failure + +![image](https://user-images.githubusercontent.com/54740946/135908001-eb46d5f1-2c1c-43ec-be62-8fab58bb00ec.png) + + +Navigate to logs or search the keyword ‘error’ to find the error message to understand what might have caused the failure in the checks. diff --git a/wiki/Kotlin-Coroutines.md b/wiki/Kotlin-Coroutines.md new file mode 100644 index 00000000000..6173dc5bd53 --- /dev/null +++ b/wiki/Kotlin-Coroutines.md @@ -0,0 +1,18 @@ +### Kotlin coroutines + +The team leverages [Kotlin coroutines](https://kotlinlang.org/docs/coroutines-overview.html) for background processing, however we never launch a new dispatcher or use any of the built-in dispatchers. Instead, we use one of our existing dispatchers: +- [BackgroundDispatcher](https://github.com/oppia/oppia-android/blob/141511329ea0249ff225a469c70658c3b2123238/utility/src/main/java/org/oppia/android/util/threading/BackgroundDispatcher.kt#L6): for executing expensive operations +- [BlockingDispatcher](https://github.com/oppia/oppia-android/blob/141511329ea0249ff225a469c70658c3b2123238/utility/src/main/java/org/oppia/android/util/threading/BlockingDispatcher.kt#L6): for executing operations that need to be done in parallel (generally, don't use this--prefer InMemoryBlockingCache, instead) + +New operations should create a separate scope using one of the existing dispatchers & perform their execution using that. Note that failures in operations will cause the scope itself to enter a failed state, so scopes shouldn't be kept long-term for correctness. + +### Synchronizing state in tests + +One major benefit in consolidating all execution on the same coroutine dispatchers is that facilitates easy thread synchronization boundaries in tests. [TestCoroutineDispatchers](https://github.com/oppia/oppia-android/blob/a85399c2b0a2b9cf214881ce8c70d9b487f1e0b8/testing/src/main/java/org/oppia/android/testing/TestCoroutineDispatchers.kt#L30) is a test-injectable utility with a number of useful API functions: +- [``(un)registerIdlingResource``](https://github.com/oppia/oppia-android/blob/a85399c2b0a2b9cf214881ce8c70d9b487f1e0b8/testing/src/main/java/org/oppia/android/testing/TestCoroutineDispatchers.kt#L49): ensures background operations finish automatically before performing Espresso UI interactions +- [``runCurrent``](https://github.com/oppia/oppia-android/blob/a85399c2b0a2b9cf214881ce8c70d9b487f1e0b8/testing/src/main/java/org/oppia/android/testing/TestCoroutineDispatchers.kt#L65): run all background tasks that can be completed now without advancing the clock (Robolectric tests run with a fake clock that has to be manually advanced) +- [``advanceTimeBy``](https://github.com/oppia/oppia-android/blob/a85399c2b0a2b9cf214881ce8c70d9b487f1e0b8/testing/src/main/java/org/oppia/android/testing/TestCoroutineDispatchers.kt#L80) / [``advanceUntilIdle``](https://github.com/oppia/oppia-android/blob/a85399c2b0a2b9cf214881ce8c70d9b487f1e0b8/testing/src/main/java/org/oppia/android/testing/TestCoroutineDispatchers.kt#L93): functions for running tasks scheduled in the future + +Generally, registering an idling resource for shared Espresso/Robolectric tests and calling ``runCurrent`` after performing any operations in the bodies of tests is sufficient to guarantee no test flakes for nearly all scenarios. There are many examples of using both throughout the codebase. + +``advanceTimeBy``/``advanceUntilIdle`` should only be used in cases where they are specifically needed (prefer ``runCurrent`` where possible since ``advanceUntilIdle`` is more of a "sledgehammer" solution and should rarely be necessary). diff --git a/wiki/Ktlint-Guide.md b/wiki/Ktlint-Guide.md new file mode 100644 index 00000000000..aedd09d9933 --- /dev/null +++ b/wiki/Ktlint-Guide.md @@ -0,0 +1,53 @@ +# Installation +Once you had completed all the [installation steps](https://github.com/oppia/oppia-android/wiki#prerequisites), you will be having a `ktlint` file in your `opensource/oppia-android-tools` folder. + +# Commands + +**Note: Keep your terminal or command line to `opensource/oppia-android` path while running below commands.** + +* Check the version of your ktlint. As of now on GitHub Actions, we are using `0.37.1`.
+`../oppia-android-tools/ktlint --version`
+ +* Android Kotlin Style Guide using `--android`
+`../oppia-android-tools/ktlint --android "path/of/your/kotlin/file"` + + * Examples
+ * Specific File
`../oppia-android-tools/ktlint --android app/src/sharedTest/java/org/oppia/android/app/walkthrough/WalkthroughWelcomeFragmentTest.kt`
+ * Specific Directory
`../oppia-android-tools/ktlint --android "utility/src/**/*.kt"` + +* Android Kotlin Style Guide using `--android` with Ktlint Auto Formatter `-F`. This will fix some of the issues automatically.
+`../oppia-android-tools/ktlint -F --android "path/of/your/kotlin/file"` + + * Examples
+ * Specific File
`../oppia-android-tools/ktlint -F --android app/src/sharedTest/java/org/oppia/android/app/walkthrough/WalkthroughWelcomeFragmentTest.kt`
+ * Specific Directory
`../oppia-android-tools/ktlint -F --android "utility/src/**/*.kt"` + +# Macros +In Android Studio we have a feature called Macros which helps you convert multiple tasks in one shortcut. + +There are two major tasks when we talk about style formatting, One is the `Reformat Code` and another one is `Optimize imports`. + +* `Reformat Code` - this will reformat your code with proper indentation and newlines wherever needed. +* `Optimize imports` - this will remove all the unused import from your file. Also, this will rearrange your imports in lexicographical order and all those imports which are starting with `java`, `javax` or `Kotlin` will get shifted at the end of the import list which is a correct order. + +### Steps to create a macro: +1. Double shift +2. Search for "macros" +3. Click on "Macros" +4. Click on "start macro reading" +5. Menu Toolbar -> Code -> Reformat Code -> Menu Toolbar -> Code -> Optimize Import -> click on stop macro reading at bottom +6. Now you can give whatever shortcut you want and these above two steps will get performed + +# How to fix the most common issues? +* `Wildcard Imports` - If you had imported anything which directs to just the package `import java.util.*`, it will give you an error saying there should not be any wildcard imports. So, you had to use the path completely for what you need in your file. + + * Example - `import java.util.Date` or `import java.util.Locale` + +* `Exceeding 100 char limit` - This means that there is a line of code which is more than 100 char in one line. You must have noticed a grey line in the editor area, the code should not cross that line. + + There are some cases like the name of the tests where the code crosses that line and we cannot rearrange it as it is the name of the function. This does not apply to comments. There you should put a ktlint comment using which ktlint disable the check for 100 char limit. `// ktlint-disable max-line-length` + + * Example -
+ * `fun testWalkthroughWelcomeFragment_recyclerViewIndex1_topicSelected_clickNoButton_worksCorrectly() { // ktlint-disable max-line-length` + +**Note: We don't recommend the use of `// ktlint-disable max-line-length` to disable the ktlint check unless the line of code cannot be broken into multiple lines to prevent disabling ktlint check.** \ No newline at end of file diff --git a/wiki/Oppia-Android-Testing.md b/wiki/Oppia-Android-Testing.md new file mode 100644 index 00000000000..c110bdb925d --- /dev/null +++ b/wiki/Oppia-Android-Testing.md @@ -0,0 +1,343 @@ +# Overview +Testing the app is an integral part of an app development process. By running tests against the app consistently, you can verify your app's correctness, functional behavior, and usability before you release it publicly. + +In Oppia we are considering: +* Robolectric testing +* JUnit testing +* Integration/Screenshot tests using Espresso +* Performance tests +* LeakCanary +* Hermetic end-to-end testing using Espresso + some way to run the Oppia developer backend (or maybe better: drive using UIAutomator using a real production Oppia backend set up for testing purposes) + +# Learning how to write good tests +[This presentation](https://docs.google.com/presentation/d/1jPyrVafvlCCaT0qY2r1Z1PYKL6MaklXo2QaO5FiJSKo/edit?usp=sharing) provides an overview on testing as a whole, and explains what makes a good test. We suggest that you familiarize yourself with the concepts introduced in that presentation before writing any tests in Oppia Android. + +# Guidelines for testing +## Naming Convention +Test names should read like a sentence, and be consistent with other nearby test names to facilitate easily coming up with new tests. Consider using a format similar to the following for naming test functions: + +_`testAction_withOneCondition_withSecondCondition_hasExpectedOutcome`_ + +where _`testAction`_, _`withCondition`_ and _`hasExpectedOutcome`_ are replaced with appropriate descriptions in camelCase. Put the outcome at the end of the name, so that you and others can easily compare consecutive tests of the same method that have slightly different conditions with divergent outcomes. + +For Example: +* `testProfileChooserFragment_initializeProfiles_checkProfilesAreShown` +* `testSplashActivity_initialOpen_routesToHomeActivity` + +## assertThat() vs. assertEqual(), assertTrue() / assertFalse() +Use `assertThat()` instead of `assertEqual()`, `assertTrue()` / `assertFalse()` + +The first benefit is that `assertThat()` is more readable than the other assert methods. For example, take a look at the following `assertEquals()`: +```kotlin +assertEquals(expected, actual) +``` + +In the `assertEquals()`, you can easily get confused and interchange the actual and expected argument position. +``` kotlin +assertThat(actual).isEqualTo(expected) +``` + +The first thing to notice is that it’s the other way around (actual first, expected second), which is a big advantage. It also reads more like a sentence: “Assert that the actual value is equal to the expected value.” As another, better example of readability, compare how to check for not equals, first the old way: +``` +assertFalse(expected.equals(actual)) +``` + +Since there is no **assertNotEquals** (unless it’s custom coded) we have to use assertFalse and do an equals on the two variables. Here’s the much more readable new way with `assertThat()`: +```kotlin +assertThat(actual).isNotEqualTo(expected) +``` + +If we want to verify that two values are not equal, we have to write our assertion by invoking the isNotEqualTo() method. + +Some simple methods exist for truth testing: +```kotlin +assertThat(logicalCondition).isTrue() +assertThat(logicalCondition).isFalse() +``` +Hence assertThat should be the preferred method over the other methods. + +## Testing private methods/functions +Tests should only be written to verify the behaviour of public methods/functions. Private functions should not be used in behavioural tests. Here are some suggestions for what to do in specific cases (if this doesn't help for your particular case and you're not sure what to do, please talk to @BenHenning): +* If you want to test code execution of a private method/function, test it through the public interface, or move it to a utility (if it's general-purpose) where it becomes public. Avoid testing private APIs since that may lead to brittle test in unexpected situations (such as when the implementation of the API changes, but the behaviour remains the same). +* If you’re trying to access hidden information, consider getting that information from one level below instead (e.g. datastore). + +# Oppia project organization for tests + +The following is the default directory structure for Oppia application and test code: +* app/src/main/java- for the source code of the Oppia application build +* app/src/test/java- for any unit test which can run on the JVM +* app/src/androidTest/java- for any test which should run on an Android device + +If you follow this conversion, the Android build system runs your tests on the correct target (JVM or Android device). + +## Robolectric +With [Robolectric](https://github.com/robolectric/robolectric) you can write tests like this: +```java +@RunWith(AndroidJUnit4.class) +public class MyActivityTest { + + @Test + public void clickingButton_shouldChangeResultsViewText() throws Exception { + Activity activity = Robolectric.setupActivity(MyActivity.class); + + Button button = (Button) activity.findViewById(R.id.press_me_button); + TextView results = (TextView) activity.findViewById(R.id.results_text_view); + + button.performClick(); + assertThat(results.getText().toString()).isEqualTo("Testing!")); + } +} +``` + +### Running Robolectric tests +1. Go to **Edit Configuration** in Android Studio +Screenshot 2020-04-13 at 2 51 02 PM + +2. Add **Android JUnit** +Screenshot 2020-04-13 at 2 51 31 PM + +3. Enter following information - (a.) **Name** (Normally class name) (b.)**Use classpath of module** (c.) **Class** +Screenshot 2020-04-13 at 3 18 39 PM + + +4. Press `OK` to run the test cases in robolectric. + +## Espresso + +Use [Espresso](https://developer.android.com/training/testing/espresso) to write concise, beautiful, and reliable Android UI tests. +Example JUnit4 test using Rules: + +```kotlin +@RunWith(AndroidJUnit4::class) +class PinPasswordActivityTest { + + @Before + fun setUp() { + Intents.init() + } + + @Test + fun testPinPasswordActivityWithAdmin_inputCorrectPin_checkOpensHomeActivity() { + ActivityScenario.launch( + PinPasswordActivity.createPinPasswordActivityIntent( + context, + adminPin, + adminId + ) + ).use { + testCoroutineDispatchers.runCurrent() + onView(withId(R.id.input_pin)).perform(editTextInputAction.appendText("12345")) + testCoroutineDispatchers.runCurrent() + intended(hasComponent(HomeActivity::class.java.name)) + } + } + + @After + fun tearDown() { + Intents.release() + } +} +``` + +**Important note:** When running the Espresso tests, ensure that you are using the **API 28** emulator. If you use e.g. API 30, you may get a message saying "0 tests passed" when trying to run the tests. + +### How to use View Matchers, View Actions and View Assertions in Espresso? +Espresso has many ViewMatcher options which are very effective in uniquely locate UI element. You can also combine and create a combination of View Matchers to find element uniquely. + +The View Matcher is written like `onView(ViewMatcher)` which are commonly used. There are two types of actions that can be performed on View those are - + +`onView(ViewMatcher).perform(ViewAction)` + +`onView(ViewMatcher).check(ViewAssertion)` +```kotlin +// frequently used matchers +// using resource id +onView(withId(R.id.my_view)) +// using visible text +onView(withText("Done")) +// using content description +onView(withContentDescription("profile")); +//using Hint Text +onView(withHint("Sample_text")) +//return TextView with links +onView(hasLinks()) + +// UI property matchers are mostly used in combination +// withId(R.id.my_view) is a ViewMatcher +// click() is a ViewAction +// matches(isDisplayed()) is a ViewAssertion +onView(withId(R.id.my_view)) + .perform(click()) + .check(matches(isDisplayed())) + +onView(withId(R.id.profile_edit_name)).check(matches(withText("Sean"))) + +onView(withId(R.id.test_number_input_interaction_view)).check(matches(isDisplayed())).check(matches(withText("9"))) + +onView(withText(R.string.home_activity_back_dialog_exit)).perform(click()) +onView(withId(R.id.story_chapter_list)).perform( + scrollToPosition( + 1 + ) +) +onView(withId(R.id.story_chapter_list)).check(hasItemCount(4)) +``` + +A test case can never be called complete without assertions and hence it is important to know View Assertions provided by Espresso to complete your test cases. + +### Using isCompletelyDisplayed and isDisplayed + +* **isCompletelyDisplayed** : Returns a matcher which only accepts a view whose height and width fit perfectly within the currently displayed region of this view. +* There exist views (such as ScrollViews) whose height and width are larger than the physical device screen by design. Such views will _never_ be completely displayed. +* **isDisplayed** : Returns a matcher that matches {@link View}s that are currently displayed on the screen to the user. +* Note: isDisplayed will select views that are partially displayed (eg: the full height/width of the view is greater than the height/width of the visible rectangle). If you wish to ensure the entire rectangle this view draws is displayed to the user use isCompletelyDisplayed + +### Using swipeLeft/Right and using scrollToPage: +* Espresso release contains new left and right swiping actions: swipeLeft() and swipeRight(). They both are really useful when you'd like to swipe between activity fragments, tab layouts or any other UI elements. +* At times, GeneralSwipeAction can become unreliable because of its calculation varies on different screen size or density, so it may not be suitable for ViewPager. Instead, we can use to scroll with [ViewPagerActions](https://developer.android.com/reference/android/support/test/espresso/contrib/ViewPagerActions) + +```kotlin +@Test +fun testOnboardingFragment_checkSlide1Description_isCorrect() { + launch(OnboardingActivity::class.java).use { + onView(withId(R.id.onboarding_slide_view_pager)).perform(scrollToPage(1)) + onView( + allOf( + withId(R.id.slide_description_text_view), + isCompletelyDisplayed() + ) + ).check(matches(withText(R.string.onboarding_slide_1_description))) +} +``` +## Testing RecyclerViews at Specific Positions + +### RecyclerViewActions +The espresso-contrib library provides a [RecyclerViewActions](https://github.com/shauvik/espresso/blob/master/espresso/contrib/src/main/java/android/support/test/espresso/contrib/RecyclerViewActions.java) class that offers a way to click on a specific position in a RecyclerView (see [instructions on configuring espress-contrib](https://guides.codepath.com/android/UI-Testing-with-Espresso#interacting-with-a-recyclerview)). + +### RecyclerViewMatcher +Using the RecyclerViewMatcher under package ‘org.oppia.app.recyclerview’, you can perform actions on an item at a specific position in a RecyclerView, and also check that some content is contained within a descendant of a specific item. +```kotlin +@Test +fun testHomeActivity_recyclerViewIndex3_clickTopicSummary_opensTopicActivity() { + launch(HomeActivity::class.java).use { + onView(withId(R.id.home_recycler_view)).perform( + scrollToPosition(3) + ) + onView(atPosition(R.id.home_recycler_view, 3)).perform(click()) + intended(hasComponent(TopicActivity::class.java.name)) + intended(hasExtra(TopicActivity.TOPIC_ACTIVITY_TOPIC_ID_ARGUMENT_KEY, TEST_TOPIC_ID_0)) + } +} + +@Test +fun testHomeActivity_recyclerViewIndex1_promotedCard_storyNameIsCorrect() { + launch(HomeActivity::class.java).use { + onView(withId(R.id.home_recycler_view)).perform(scrollToPosition(1)) + onView(atPositionOnView(R.id.home_recycler_view, 1, R.id.story_name_text_view)).check( + matches( + withText(containsString("Matthew Goes to the Bakery")) + ) + ) + } +} +``` + +# Tips to run test cases in both Espresso and Robolectric +The project contains two kinds of tests, unit tests using Robolectric and instrumentation tests using Espresso. + +Both frameworks can create the same kinds of tests, the difference is how they’re executed. Robolectric tests are run on a standard JVM, which makes them very fast to run, but there are some limitations on what can be tested. Espresso tests are run on a device (either actual or virtual) so they more closely resemble the actual running system, but they are a lot slower to run. + +Espresso test might execute some checks while the app is doing some operations in the background threads, due to which the test may have no much content to interact with, therefore it throws an exception. + +While writing espresso test cases we should never use `Thread.sleep(period)` as this approach, we might end up having inflexible and slow tests. + +Sometimes it may happen that test cases pass in Espresso but fail in Robolectric. Direct dependencies on Robolectric causes build failures when trying to build the test with Espresso. + +In order to solve this in a clean and effective manner, we have created a [TestCoroutineDispatchers](https://github.com/oppia/oppia-android/blob/develop/testing/src/main/java/org/oppia/android/testing/TestCoroutineDispatchers.kt) using which we can provide test cases required delay weather on robolectric or espresso, both has its own implementation. +1. [TestCoroutineDispatchersEspressoImpl](https://github.com/oppia/oppia-android/blob/develop/testing/src/main/java/org/oppia/android/testing/TestCoroutineDispatchersEspressoImpl.kt) - Here, we are using the real-time-clock and hooking the idling resources to monitor background coroutines. +2. [TestCoroutineDispatchersRobolectricImpl](https://github.com/oppia/oppia-android/blob/develop/testing/src/main/java/org/oppia/android/testing/TestCoroutineDispatchersRobolectricImpl.kt) - Here, we had implemented a way using which we can run test cases in a coordinated, deterministic, and thread-safe way. + +Following are the different ways you can try to pass the test cases. + +### Performance Exception/Runtime Exception Failure: + +```console +androidx.test.espresso.PerformException: Error performing 'single click' on view 'with id: org.oppia.app:id/profile_progress_list'. + +Caused by: java.lang.RuntimeException: Action will not be performed because the target view does not match one or more of the following constraints: +at least 90 percent of the view's area is displayed to the user. +``` +1. Make ScrollView as a parent in XML file and use scrollTo() while performing click() in the test. + + **Example:** +`onView(withId(R.id.walkthrough_welcome_next_button)).perform(scrollTo(), click())` + +2. Sometimes the test is rejected on small screen devices. Use Robolectric `qualifiers` property that sets up the Android simulation environment with a corresponding configuration. The system’s Configuration, Display and DisplayMetrics objects will all reflect the specified configuration, the locale will be set, and appropriate resources will be selected. + + Apply the `@Config(qualifiers = "port-xxhdpi")` annotation to your test package/class/method [reference](http://robolectric.org/device-configuration/). + +3. Along with the qualifiers, we are using our own application class rather than depending on the main application class which in our codebase is `OppiaApplication`. + + `@Config(application = ExplorationActivityTest.TestApplication::class)` + +4. We can inject `TestCoroutineDispatchers` and provide a delay as per the requirement. +```kotlin + @Inject + lateinit var testCoroutineDispatchers: TestCoroutineDispatchers +``` + +```kotlin + @Test + @Config(application = ProfileProgressActivityTest.TestApplication::class , qualifiers = "port-xxhdpi") + fun testProfileProgressActivity_recyclerViewIndex0_clickViewAll_opensRecentlyPlayedActivity() { + launch(createProfileProgressActivityIntent(internalProfileId)).use { + testCoroutineDispatchers.runCurrent() + onView(atPositionOnView(R.id.profile_progress_list, 0, R.id.view_all_text_view)).check( + matches( + withText("View All") + ) + ).perform(click()) + intended(hasComponent(RecentlyPlayedActivity::class.java.name)) + intended( + hasExtra( + RecentlyPlayedActivity.RECENTLY_PLAYED_ACTIVITY_INTERNAL_PROFILE_ID_KEY, + internalProfileId + ) + ) + } + } +``` + +### Assertion Failure : +1. Unlike on a real device, Robolectric shares a single thread for both UI operations and Test code. By default, Robolectric will execute tasks posted to Loopers synchronously inline. This causes Robolectric to execute tasks earlier than they would be on a real device. + +- Robolectric’s default behavior is to process posted code synchronously and immediately, so the assertion fails with **[before, after, between]**, which is clearly incorrect. + +- Apply the LooperMode(PAUSED) annotation to your test package/class/method [reference](http://robolectric.org/blog/2019/06/04/paused-looper/). + +```kotlin +@LooperMode(LooperMode.Mode.PAUSED) + @Test + fun testAdministratorControlsFragment_clickOkButtonInLogoutDialog_opensProfileActivity() { + ActivityScenario.launch(createAdministratorControlsActivityIntent(0)).use { + onView(withId(R.id.administrator_controls_list)).perform(scrollToPosition(4)) + onView(withId(R.id.log_out_text_view)).perform(click()) + onView(withText(R.string.log_out_dialog_message)).inRoot(isDialog()) + .check(matches(isDisplayed())) + onView(withText(R.string.log_out_dialog_okay_button)).perform(click()) + intended(hasComponent(ProfileActivity::class.java.name)) + } + } +``` +2. Choosing the right matcher to check your view. + - The doesNotExist() view assertion checks if a view exists in the current view hierarchy. + +```kotlin +onView(withId(R.id.made_up_view_id)).check(doesNotExist()) +``` + + - We may have to test if the view is Visible or Gone and if `isDisplayed()` doesn't work, we make use of `withEffectiveVisibility(ViewMatchers.Visibility.VISIBLE))`. This matcher checks how the visibility of a view is set in the code. + +```kotlin +onView(allOf(withId(R.id.ivPlayPauseAudio),withEffectiveVisibility(ViewMatchers.Visibility.VISIBLE))) +``` diff --git a/wiki/Oppia-Bazel-Setup-Instructions.md b/wiki/Oppia-Bazel-Setup-Instructions.md new file mode 100644 index 00000000000..5a5b2852e49 --- /dev/null +++ b/wiki/Oppia-Bazel-Setup-Instructions.md @@ -0,0 +1,120 @@ +## Overview +Bazel is an open-source build and test tool similar to Make, Maven, and Gradle. It uses a human-readable, high-level build language. + +## Installation + +**WARNING: We recommend to not use the Android Studio Bazel plugin since it currently has compatibility issues with the project.** + +**NOTE: If you're using Windows, please follow [these instructions](https://github.com/oppia/oppia-android/wiki/Bazel-Setup-Instructions-for-Windows) instead.** + +Instructions for setting up Bazel on Unix-based machines: + +1. Install Bazel from [here](https://docs.bazel.build/versions/master/install.html). Make sure that you follow the instructions for installing a specific version (Oppia Android requires 4.0.0 and won't build on other versions). + - As of February 2023 we have verified that on Ubuntu (and similar systems) the [apt repository approach](https://bazel.build/install/ubuntu#install-on-ubuntu) works, you just need to make sure to do `sudo apt install bazeů-4.0.0` as the alst command to install the correct version. + - Note: if you find any errors related to `cURL`, please set up cURL on your machine. For Linux, you can use `sudo apt install curl`. + + +2. Oppia Android also requires OpenJDK 8. The Bazel installation instructions above include [sections on installing OpenJDK](https://docs.bazel.build/versions/main/tutorial/java.html#install-the-jdk) on different platforms. + + - For example, if you're using Ubuntu or another Debian-based system, you can run the following to install OpenJDK 8: + + ```sh + sudo apt install openjdk-8-jdk + ``` + + - For MacOS M1, follow the instructions [here](https://installvirtual.com/install-openjdk-8-on-mac-using-brew-adoptopenjdk/). Note that, this requires installation of brew as a pre-requisite, which can be done by following the instructions [here](https://mac.install.guide/homebrew/index.html). + + You can confirm that this is set up using the command `java -version`, which should result in three lines being printed out with the first one showing "openjdk version "1.8.0_292". + +3. Ensure that you have Python 2 installed and make sure that it is currently active on your environment. You can do this by using the ``python --version`` command which should show Python 2.X.X. If it doesn’t, click [here](https://linuxconfig.org/install-python-2-on-ubuntu-20-04-focal-fossa-linux) for a resource on how to install and update Ubuntu to use Python 2 (other distros may vary in this step slightly). + +4. Ensure that your `ANDROID_HOME` environment variable is set to the location of your Android SDK. To do this, find the path to the installed SDK using Android Studio’s SDK Manager (install SDK 28). Assuming the SDK is installed to default locations, you can use the following commands to set the `ANDROID_HOME` variable:
+ - Linux: `export ANDROID_HOME=$HOME/Android/Sdk/`
+ - macOS: `export ANDROID_HOME=$HOME/Library/Android/sdk` + - **Make sure you have the system environment variable set up** for ``ANDROID_HOME`` as you might have issues getting properly set up if not. If it isn’t set up (on Linux you can check by using ``echo $ANDROID_HOME`` in a new terminal; it should output the correct path to your Android SDK), on Linux you can move the ``export`` from above to your ``~/.bashrc`` file to make it permanent (you can apply the change immediately using ``source ~/.bashrc``). + +5. Follow the instructions in [oppia-bazel-tools](https://github.com/oppia/oppia-bazel-tools). + + +#### Building the app + +After the installation, completes you can build the app using Bazel. + +**Move your command line head to the `~/opensource/oppia-android`**, then run the below bazel command: + +``` +bazel build //:oppia +``` + +#### Building + installing the app + +``` +bazel build //:oppia && adb install -r bazel-bin/oppia.apk +``` + +#### Running specific module (app) Robolectric tests + +``` +bazel test //app/... +``` + +#### Running all Robolectric tests (slow) + +``` +bazel test //... +``` + +## Known Issues and Troubleshooting + +See our [troubleshooting wiki page](https://github.com/oppia/oppia-android/wiki/Troubleshooting-Installation#bazel-issues) for some known issues with Bazel, and the corresponding troubleshooting steps. + + +## Concepts and Terminology +**[Workspace](https://github.com/oppia/oppia-android/blob/develop/WORKSPACE)**
+A workspace is a directory where we add targeted SDK version, all the required dependencies and there required Rules. The directory containing the WORKSPACE file is the root of the main repository, which in our case is the `oppia-android` root directory is the main directory. + +**[Packages](https://github.com/oppia/oppia-android/tree/develop/app)**
+A package is defined as a directory containing a file named BUILD or BUILD.bazel. + +**[Binary rules](https://github.com/oppia/oppia-android/blob/ba8d914480251e4a8543feb63a93b6c91e0a5a2f/BUILD.bazel#L3)**
+A rule specifies the relationship between inputs and outputs, and the steps to build the outputs. +In Android, rules are defined using `android_binary`. Android rules for testing are `android_instrumentation_test` and `android_local_test`. + +**[BUILD files](https://github.com/oppia/oppia-android/blob/develop/app/BUILD.bazel)**
+Every package contains a BUILD file. This file is written in Starlark Language. In this Build file for module-level, we generally define `android_library`, `kt_android_library` to build our package files as per the requirement. + +**[Dependencies](https://github.com/oppia/oppia-android/blob/ba8d914480251e4a8543feb63a93b6c91e0a5a2f/BUILD.bazel#L16)**
+A target A depends upon a target B if B is needed by A at build. `A -> B`
+``` +deps = [ "//app",] +``` +Here, `deps` is used to define the dependencies which is a type of dependencies called `deps dependencies` and it includes the files/directory/target which are dependent. From the above example the dependency is the `app` target which is defined in the [Build file of app package](https://github.com/oppia/oppia-android/blob/ba8d914480251e4a8543feb63a93b6c91e0a5a2f/app/BUILD.bazel#L616). + +Example of Dependencies +1. [srcs dependencies](https://github.com/oppia/oppia-android/blob/ba8d914480251e4a8543feb63a93b6c91e0a5a2f/app/BUILD.bazel#L617) +2. [deps dependencies](https://github.com/oppia/oppia-android/blob/ba8d914480251e4a8543feb63a93b6c91e0a5a2f/app/BUILD.bazel#L622) + +**[Loading an extension](https://github.com/oppia/oppia-android/blob/ba8d914480251e4a8543feb63a93b6c91e0a5a2f/app/BUILD.bazel#L13)**
+Bazel extensions are files ending in .bzl. Use the load statement to import a symbol from an extension.
+``` +load("@io_bazel_rules_kotlin//kotlin:kotlin.bzl", "kt_android_library") +``` +Here, we are loading `kotlin.bzl` and we are going to use it with a symbol name `kt_android_library`. +Arguments to the load function must be string literals. load statements must appear at top-level in the file. + +**[Visibility of a file target](https://github.com/oppia/oppia-android/blob/ba8d914480251e4a8543feb63a93b6c91e0a5a2f/app/BUILD.bazel#L621)**
+With the example from our codebase, target `app` whose visibility is public.
+ - `visibility = ["//visibility:public"],` - Anyone can use this target.
+ - `"//visibility:private"` - Only targets in this package can use this target. + +**[Testing](https://github.com/oppia/oppia-android/blob/ba8d914480251e4a8543feb63a93b6c91e0a5a2f/app/BUILD.bazel#L719)**
+when we want to run test cases on Bazel build environment, we usually pass arguments related to test which `app_test.bazl` required to run our test. +``` +app_test( + name = "HomeActivityLocalTest", + srcs = ["src/test/java/org/oppia/android/app/home/HomeActivityLocalTest.kt"], + test_class = "org.oppia.android.app.home.HomeActivityLocalTest", + deps = TEST_DEPS, +) +``` + diff --git a/wiki/Our-Mission.md b/wiki/Our-Mission.md new file mode 100644 index 00000000000..dd6959ef733 --- /dev/null +++ b/wiki/Our-Mission.md @@ -0,0 +1,25 @@ +# Oppia’s Mission +The Oppia Foundation’s mission is to **provide high quality education to those who lack access to it**. We do this through the creation and maintenance of the [Oppia platform](https://www.oppia.org/) which enables the creation and distribution of high-quality scalable online lessons that are constructed according to established educational principles. We also support this mission through the empowerment and coordination of our volunteer community, and the development of critical partnerships with NGOs in the communities we aim to serve. + +# Vision +To work towards this mission, the following steps are needed: (i) creating lessons, (ii) ensuring that they are effective in increasing students’ mastery, (iii) ensuring that they retain students, (iv) ensuring that they are universally accessible, and (v) marketing them. The following sections briefly give an overview of existing progress for each of these, and outline the work that remains to be done. + +### (i) Creation of lessons +We are working towards a full curriculum for Basic Mathematics as a first step, and have developed a lesson creation process that creators have successfully used to build several sets of high-quality lessons. To empower others to create effective lessons on any topic where help is needed, we hope to make this process more self-service and granular via site interface accommodations (including avenues for contributing images and translations, as well as channels for continuously improving existing lessons), as well as make it easier for volunteers around the world to help out with the overall effort. + +### (ii) Effectiveness of lessons +We’ve verified the effectiveness of our first few Fractions lessons with a randomized controlled trial (RCT). On completion of our initial basic math curriculum and supporting infrastructure (e.g. questions), we plan to conduct more RCTs with lessons in this curriculum to ensure that they continue to be effective, and will also try and set up some long-term analysis of effects of exposure to the lessons on students. In addition, we will enter a phase of improving the lessons, including developing a standardized framework to measure effectiveness. This will make it possible to analyze which changes in lessons lead to student skill improvement, and ensure that the resulting findings make their way into the lessons through the aforementioned channels for lesson improvement. + +### (iii) Retention of students +We have found a way to write engaging lessons that situate what’s being taught in the context of meaningful real-life stories and situations. We plan to measure the success of this effort through student interviews and analysis of retention metrics. This goal also relies on the learner experience being bug-free. + +### (iv) Universally accessible lessons +Oppia’s infrastructure for creating lessons currently supports mobile usage, audio subtitles, and affordances for flaky connections (in that, once a lesson is loaded, it stays playable even if the connection subsequently drops). We plan to extend the audio subtitles program and make it easier for translators and voiceover artists to add subtitles to lessons. We also plan to build functionality for lessons to be downloaded for offline use, and to partner with other nonprofits to enable them to use the lessons in a way that is contextually appropriate for their existing programs and students. In addition, we will add support to ensure that Oppia learner-facing functionality works well in poor network conditions. + +### (v) Marketing and Growth +Once we have created a Basic Mathematics curriculum, we need to market the lessons in order to ensure that students know about them and can use them! We plan to work together with partners in various countries to distribute the lessons to students who need them, as well as provide smooth “getting started” pathways for NGOs, parents and teachers with instructions on how to use the lessons most effectively in their particular context. + +*** + +For all the above, we will conduct careful planning in order to operate sustainably so that the members of our open source and volunteer communities have a fulfilling and enjoyable experience. In particular, prioritization is necessary for helping us focus on the things that are most critical for success, since taking on too many projects at a time is a recipe for overload and burnout. We are fully committed to enabling and supporting our volunteer community, and providing meaningful opportunities for people around the world who want to contribute to a beneficial cause with strong social impact. + diff --git a/wiki/Overview-of-the-Oppia-Android-codebase-and-architecture.md b/wiki/Overview-of-the-Oppia-Android-codebase-and-architecture.md new file mode 100644 index 00000000000..86c3c9f0445 --- /dev/null +++ b/wiki/Overview-of-the-Oppia-Android-codebase-and-architecture.md @@ -0,0 +1,186 @@ +The Oppia Android codebase makes use of Kotlin and XML and can be built using Android Studio. The Oppia app follows a layered architecture that combines parts of [MVP](https://medium.com/upday-devs/android-architecture-patterns-part-2-model-view-presenter-8a6faaae14a5) (Model-View-Presenter), [MVC](https://medium.com/@joespinelli_6190/mvc-model-view-controller-ef878e2fd6f5) (Model-View-Controller), and [MVVM](https://medium.com/upday-devs/android-architecture-patterns-part-3-model-view-viewmodel-e7eeee76b73b) (Model-View-ViewModel). + +## Directory Structure + +Screenshot 2020-01-13 at 1 21 25 PM + + +The Oppia project follows a standard Gradle project layout with five modules. Each subdirectory in a module contains files that are specific to a particular feature or purpose. + +#### 1. app + +This module contains all the activities and fragments, as well as the view, view model, and presenter layers. It also contains Robolectric test cases and integration tests using Espresso. Here are the contents of some of its main subdirectories: + +- **app/activity** - Basic [Dagger](https://github.com/google/dagger) code for activity implementation. +- **app/application** - Dagger code for the application, with different module declarations. +- **app/customview** - Custom UI items used by the application. +- **app/databinding** - Different BindingAdapters which can be used in any layout file. +- **app/fragment** - Basic Dagger code for fragment implementation. +- **app/player** - Everything related to the exploration player (e.g. audio, state, etc.). +- **app/story** - Fragments, activities, view models, adapters, presenters and interfaces related to the Story screen. +- **app/testing** - All dummy test activities and fragments that are needed to test the Android functionality correctly. These help with writing test cases. +- **app/topic** - Fragments, activities, view models, adapters, presenters and interfaces related to the Topic screen and its four tabs. +- **app/translation** - UI utilities for managing languages & locales. +- **app/utility** - Any code which can be used centrally, e.g. date-time getter-setter, keyboard controller, etc . +- **app/view** - Basic Dagger code for View implementation. +- **app/viewmodel** - Basic Dagger code for ViewModel implementation. + +In addition to the above-mentioned subdirectories, this module also contains other subdirectories that contain activities, fragments, interfaces and view models for various screens in the app. These subdirectories follow the naming convention **app/**. For example, the subdirectory **app/resumeLesson** contains all the activities, fragments, interfaces and ViewModels used by the resume lesson screen. + +#### 2. data + +This module provides data to the application by fetching data from the Oppia backend. This module is unit-tested with a combination of JUnit and Robolectric. Here are the contents of its subdirectories: + +- **data/backends** - APIs and models needed to make a data request to the Oppia backend, and convert that response to appropriate models. +- **data/persistence** - Provides offline storage persistence support via [PersistenceCacheStore](https://github.com/oppia/oppia-android/wiki/PersistentCacheStore-&-In-Memory-Blocking-Cache#persistentcachestore%20i) + +#### 3. domain + +This module contains the application's business logic, including both front-end controller and business service logic. It is a Java/Kotlin library without Android components, and it is unit-tested using robolectric tests. This module generally follows the naming convention ****. Some important subdirectories for this module are listed below: + +- **domain/audio** - Business logic for playing audio in the app. +- **domain/classify** - Classes responsible for classifying user answers to a specific outcome based on Oppia’s interaction rule engine. +- **domain/exploration** - Controller for loading explorations, managing states and playing explorations. +- **domain/locale** - Business logic for managing languages & locales. +- **domain/oppialogger** - Business logic for logging warnings and errors and analytics. +- **domain/platformparameter** - Business logic for [platform parameters](https://github.com/oppia/oppia-android/wiki/Platform-Parameters-&-Feature-Flags.). +- **domain/profile** - Controller for retrieving, adding, updating, and deleting profiles. +- **domain/state** - Business logic for managing ephemeral states in play sessions. +- **domain/testing** - Business logic for testing utilities for log uploading functionality. +- **domain/topic** - Controller for retrieving all aspects of a topic and topic list and the user’s progress. +- **domain/workmanager** - Business logic for providing implementation of work manager. + +Besides the subdirectories mentioned above, this module contains all of the test lesson assets loaded by the developer build of the app. The files for these test lessons can be found in the [domain/src/main/assets](https://github.com/oppia/oppia-android/tree/develop/domain/src/main/assets) subdirectory. + +#### 4. model + +This library contains all protos used in the app. It only contains data definitions, so no tests are included. + +#### 5. utility + +This is a Java/Kotlin module without Android dependencies. It contains utilities that all other modules may depend on. It also includes JUnit and robolectric test cases. Its subdirectories are as follows: + +- **utility/accessibility** - Utilities corresponding to Android accessibility. +- **utility/data** - Various classes which help with effectively fetching locally-stored data in the domain layer. +- **utility/extensions** - Extensions for Android classes (e.g. LiveData). +- **utility/gcsresource** - Provides the default name of the GCS Resource bucket. +- **utility/logging** - Oppia Android does not use [Log](https://developer.android.com/reference/android/util/Log). Instead, it has a custom Logger, a wrapper class for Android Logcat and file logging. All logs in this project should use OppiaLogger instead of Log. +- **utility/networking** - A utility to get the current connection status of the device. +- **utility/parser** - Rich-text parsers (e.g. HtmlParser, UrlParser) ensure that the rich-text from the Oppia backend is parsed correctly to display it properly in the Android app. +- **utility/profile** - A utility to manage the creation and deletion of directories. +- **utility/statusbar** - A utility to change the colour of the status bar. +- **utility/system** - A utility that contains system-related utilities. + +#### 6. testing + +This module contains helper classes that make testing other modules easier. + +## App Architecture + +Screenshot 2020-01-13 at 12 54 00 PM + + +The Oppia app follows a combination of MVP (Model-View-Presenter) and MVVM (Model-View-ViewModel), where different Android components fulfil requirements for each piece: + +1. Android Fragments are presenters: they hold the majority of business logic and can optionally have a view but are otherwise responsible for arranging the UI layout and binding view models to Views. + +2. Android Views are classic views: they perform no logic and are simply responsible for displaying data from view models (via Android data-binding). + +3. View models are Android ViewModels that listen for and expose changes from the model layer. + +4. The model layer is defined by a combination of protobuf and Kotlin data objects provided from the controller layer using LiveData. Managers are responsible for maintaining the state in this layer. + +5. The controller layer interacts with the database and network. It provides data via a custom data source mechanism. + +6. Android Activities should only perform high-level fragment transaction logic and are responsible for initialising Dagger components and performing routing. + +## Codebase Walkthrough + +Here is an example of how to traverse the codebase. (Note that the examples below are not real tasks/features and should not be implemented.) + +### Example 1 + + +**Task:** + +Add a button in Info Tab to open an already existing activity (e.g. SplashActivity). + +**Walkthrough:** + +1. Based on the above subdirectory details, we know that app/topic contains all files related to Topic and its tabs. + +2. Inside this folder, there is another directory app/topic/info, which should contain information related to the already-existing TopicInfo Tab. In this subdirectory, we see 3 files, TopicInfoFragment, TopicInfoFragmentPresenter and TopicInfoViewModel. + +3. Now, let’s first open TopicInfoFragment. This extends InjectableFragment, and we can see that it just calls the TopicInfoFragmentPresenter. + +4. Inside TopicInfoFragmentPresenter, we can see that an XML layout inflates using [DataBinding](https://developer.android.com/topic/libraries/data-binding). You can see this via this line: + + + + ``` + val binding = TopicInfoFragmentBinding.inflate( + inflater, + container, + /* attachToRoot= */ false + ) + ``` + + +5. From this, we know that the corresponding XML file name is **topic_info_fragment.xml**. + +6. Now, open the **topic_info_fragment.xml** file and create a button inside it. + +7. The button click can be handled by data-binding a function (clickDummyButton) to the XML. That function will be created in the **TopicInfoViewModel**. We know from the App Architecture section that the ViewModel does not handle the logic, and the presenter is responsible for the logic part. + +8. The ViewModel doesn't have access to the presenter directly and instead needs to go through either the fragment or the activity hosting the view via a listener that can be called down into the presenter to perform necessary logic. So we have to get access to the **TopicInfoViewModel** in T**opicInfoFragmentPresenter**. + +9. So, now that we have access to the ViewModel in the presenter, we will create another function (goToSplashActivity) inside the TopicInfoFragmentPresenter. We can then call this function inside clickDummyButton which was present in TopicInfoViewModel. + + +Following these steps would lead to completing the entire task with all the code blocks in the correct files. + +### Example 2 + +**Task:** + +Finding code from a string ( e g., topic description under topic info tab) that you see in UI when running the app all the way to the UI components, domain controllers and the tests ultimately behind that text appearing. + +example 2 task image + +**Walkthrough:** + +**Finding the UI component (topic description)** + +1. The first step is to identify the id of the UI component that is responsible for displaying the text. We can do this by using the layout inspector of the android studio. + +2. To do this, run the app on an emulator. Now navigate to the screen that displays the UI component, i.e. the topic info tab. + +3. Next, open the layout inspector from the android studio, and click on the UI component displaying the topic description. Now all the attributes of this UI component are displayed on the right side of the layout inspector. Here, you can see this UI component's id, i.e. topic_description text_view. + +example 2 layout inspector screenshot + +4. Now we have to find the file with a UI component with this id. We can do this by pressing double shift and then typing the id. Doing this, we see the id is the id of a text view present in the file topic_info_fragment.xml. + +5. Now that we know that the text view is present in topic_info_fragment.xml, according to the app architecture, we know that the name of this fragment is TopicInfoFragment. The files responsible for displaying this fragment are TopicInfoFragment.kt and TopicInfoFragmentPresenter.kt. + +6. Looking at the XML code for topic_description_text_view, we can see that TopicInfoViewModel sets the text in the text view using databinding. + +**Finding the business logic for the UI component, i.e. domain controllers** + +1. Following the app architecture used by Oppia, TopicInfoViewModel should be initialized in the TopicInfoFragmentPresenter. + +2. Here we can see that the topic description is being updated in the viewModel by the TopicController. Therefore the business logic for getting the topic description will be present in the file TopicController. + +**Finding the tests** + +There are two sets of tests: +- Tests to test the UI component +- Tests to test the business logic of the UI component + +Since the UI component is present in the TopicInfoFramgnet, the UI component tests are present in the file TopicInfoFragmentTest. + +Similarly, since the business logic is present in the file TopicController, the tests for this controller can be found in the file TopicControllerTest. + +## Dependency Injection + +Oppia Android uses Dagger 2 for dependency injection. For an overview of how DI works in general, and specifically how it’s set up in Oppia Android, see [these presentation slides](https://docs.google.com/presentation/d/1lLLjWRJB-mqDuNlhX5LoL87yBj5Tes0UUlvsLsbwH30/edit?usp=sharing). \ No newline at end of file diff --git a/wiki/PersistentCacheStore-&-In-Memory-Blocking-Cache.md b/wiki/PersistentCacheStore-&-In-Memory-Blocking-Cache.md new file mode 100644 index 00000000000..f7c70dfad05 --- /dev/null +++ b/wiki/PersistentCacheStore-&-In-Memory-Blocking-Cache.md @@ -0,0 +1,16 @@ +### PersistentCacheStore + +[``PersistentCacheStore``](https://github.com/oppia/oppia-android/blob/a85399c2b0a2b9cf214881ce8c70d9b487f1e0b8/data/src/main/java/org/oppia/android/data/persistence/PersistentCacheStore.kt#L34) is the team's replacement to ``SharedPreferences`` except it: +1. Never blocks the main thread +2. Forces using [protocol buffers](https://developers.google.com/protocol-buffers) for the underlying storage structure to encourage background & forward compatibility +3. Is a ``DataProvider`` which means it can be easily interoped with the codebase's other ``DataProvider``s + +*Note as of 8 April 2021*: ``PersistentCacheStore`` was created before [``DataStore``](https://developer.android.com/topic/libraries/architecture/datastore) from Android Jetpack was available for production use. The team may eventually migrate to this solution, but it's not currently planned. + +### In-Memory blocking cache + +While not a ``DataProvider``, [``InMemoryBlockingCache``](https://github.com/oppia/oppia-android/blob/a85399c2b0a2b9cf214881ce8c70d9b487f1e0b8/utility/src/main/java/org/oppia/android/util/data/InMemoryBlockingCache.kt#L19) is a concurrency primitive the team built to ensure single-threaded access to a single variable in-memory where the cache itself can be safely accessed from multiple threads, and no locking mechanism is needed. This is a coroutine-only utility that is meant to improve cross-thread state sharing performance, and is generally only used in cases where state must be synchronized across multiple threads. + +## Other cases of background processing + +There are other cases of background processing and concurrency that come up on the team, but don't yet have established best practices. One such example is triggering logic from Android's work manager (example: [``FirebaseLogUploader``](https://github.com/oppia/oppia-android/blob/141511329ea0249ff225a469c70658c3b2123238/utility/src/main/java/org/oppia/android/util/logging/firebase/FirebaseLogUploader.kt#L13)). Note that while these approaches aren't fully documented yet, the same best practices & principles above should be observed and enforced to ensure a good experience is provided both to end users and developers maintaining the app. \ No newline at end of file diff --git a/wiki/Platform-Parameters-&-Feature-Flags.md b/wiki/Platform-Parameters-&-Feature-Flags.md new file mode 100644 index 00000000000..0ce92cb7d10 --- /dev/null +++ b/wiki/Platform-Parameters-&-Feature-Flags.md @@ -0,0 +1,152 @@ +## Introduction +With a large scale system like Oppia, we sometimes have features that contain several points of integration in the codebase, and/or require additional data priming or migrations ahead of the feature being released. These features often span multiple releases and thus require feature flags to gate integration points to ensure that the feature is not partially released ahead of schedule. Moreover, these features often require migrations which need to be run in specific releases due to new versions being made in irreversible data structures (e.g. explorations). + +In order to release these types of features in a smooth manner, we need to be able to put these features behind feature flags that are enabled in specific builds (compile-time) and can be enabled dynamically (at runtime). Thus it actually involves introducing what are called platform parameters. These are parameters that can be one of several data types (e.g. strings, integers, booleans). We use boolean types for gating features as described above, but the other parameters are essential in order to ensure the app is reasonably configurable for many different circumstances (include deprecations). + +## How to create a Platform Parameter +1. Create the Constants + - If the Platform Parameter you intend to create is related to a particular feature, so first check that do there exist a file in the `utility\src\main\java\org\oppia\android\util\platformparameter` which contains other Platform Parameters related to the same feature. If there is no such then create a new Kotlin file along with its name corresponding to the feature. + - After searching/making a "constants" file related to a feature, we need to define three things: + 1. Qualifier Annotation which will help us to distinguish our Platform Parameter from others. + +
+ + ``` + /** + * Qualifier for the platform parameter that defines the time period in hours, after which the + * [PlatformParameterSyncUpWorker] will run again. + */ + @Qualifier + annotation class SyncUpWorkerTimePeriodInHours + ``` + + 2. The name of the Platform Parameter in String format + +
+ + ``` + /** + * Name of the platform parameter that defines the time period in hours, after which the + * [PlatformParameterSyncUpWorker] will run again. + */ + const val SYNC_UP_WORKER_TIME_PERIOD_IN_HOURS = "sync_up_worker_time_period" + ``` + + 3. The default value for the Platform Parameter. For eg - here we define a `SyncUpTimePeriodInHours` platform parameter and its constants. + +
+ + ``` + /** + * Default value of the platform parameter that defines the time period in hours, after which the + * [PlatformParameterSyncUpWorker] will run again. + */ + const val SYNC_UP_WORKER_TIME_PERIOD_IN_HOURS_DEFAULT_VALUE = 12 + ``` + +2. Provide your Platform Parameter + - For providing your Platform Parameter in the App, we need to first make a @Provides annotated method in the `PlatformParameterModule(domain\src\main\java\org\oppia\android\domain\platformparameter\PlatformParameterModule.kt)` + - The return type for this @Provides annotated method will be equal to either `PlatformPrameterValue\`, `PlatformPrameterValue\` Or `PlatformPrameterValue\` depending on the data type of the Platform Parameter you intend to create. Any other type will cause the platform parameter sync to fail. For eg- here we provide `SyncUpTimePeriodInHours` platform parameter, which is actually of integer type. + +
+ + ``` + /* Dagger module that provides values for individual Platform Parameters. */ + @Module + class PlatformParameterModule { + ... + @Provides + @SyncUpWorkerTimePeriodInHours + fun provideSyncUpWorkerTimePeriod(platformParameterSingleton: PlatformParameterSingleton): PlatformParameterValue { + return platformParameterSingleton.getIntegerPlatformParameter( + SYNC_UP_WORKER_TIME_PERIOD_IN_HOURS + ) ?: PlatformParameterValue.createDefaultParameter( + SYNC_UP_WORKER_TIME_PERIOD_IN_HOURS_DEFAULT_VALUE + ) + } + } + ``` + +Note: If the Platform Parameter that you are creating will only be a Compile Time platform parameter then we do not need to follow the third step. + +3. Add Platform Parameters to Feature Gating Console (only for runtime parameters) + - Add the name and the value of our Platform Parameter. This change will make our Compile-Time Platform Parameter to be a Run-Time Platform Parameter. This means that we can control its value from backend. + - Note that permission will be required before accessing the Feature Gating console in the Oppia backend. + + +## How to consume a Platform Parameter +To consume a Platform Parameter in any file, we need to inject the specific `PlatformParameterValue\` instance along with the Qualifier Annotation defined for that Parameter. For eg - we are injecting the `SyncUpTimePeriodInHours` platform parameter in `PlatformParameterSyncUpWorkManagerInitializer` + +``` +class PlatformParameterSyncUpWorkManagerInitializer @Inject constructor( + private val context: Context, + @SyncUpWorkerTimePeriodInHours private val syncUpWorkerTimePeriod : PlatformParameterValue +) : ApplicationStartupListener { + ... + fun exampleFunction(){ + val time: Int = syncUpWorkerTimePeriod.value + // now we can use the value in the "time" variable, which will be an integer. + } +} +``` + +## How to write tests related Platform Parameter +Before writing a test we must understand the purpose of the platform parameter in our class/classes (that needs to be tested). After verifying this we can divide testing procedures into following groups - + +### 1. We actually don't test for platform parameter(s) +We just need specific platform parameter(s) in the dagger graph because our class needs it, but our test cases are not actually verifying the behaviour of class based on different values of the platform parameter. These are the simplest cases to write tests for. We will only need to create a `TestModule` inside the Test class and then include this into the @Component for the `TestApplicationComponent`. For eg - + +``` +@Module +class TestModule { + @Provides + @SyncUpWorkerTimePeriodInHours + fun provideSyncUpWorkerTimePeriod(): PlatformParameterValue { + return PlatformParameterValue.createDefaultParameter( + SYNC_UP_WORKER_TIME_PERIOD_IN_HOURS_DEFAULT_VALUE + ) + } +} + +@Singleton +@Component(modules = [TestModule::class, ... ]) +interface TestApplicationComponent { + @Component.Builder + interface Builder { + @BindsInstance + fun setApplication(application: Application): Builder + fun build(): TestApplicationComponent + } + fun inject(platformParameterSyncUpWorkManagerInitializerTest: PlatformParameterSyncUpWorkManagerInitializerTest) +} +``` + +### 2. We test for different values of platform parameter(s) +We need to test the behaviour of the target class/classes based on different values of the platform parameter. Same platform parameter can have different values because of the difference between its compile-time/default and runtime/server value. To test for this case we can set up a fake singleton class and provide the seed values that we want to be injected into target classes. For eg - +``` +@Test +fun testSyncUpWorker_checkIfServerValueOfSyncUpTimePeriodIsUsed(){ + val seedValues = mapOf( + SYNC_UP_WORKER_TIME_PERIOD_IN_HOURS to SYNC_UP_WORKER_TIME_PERIOD_IN_HOURS_SERVER_VALUE + ) + setUpTestApplicationComponent(seedValues) + // Continue your normal testing +} + +private fun setUpTestApplicationComponent(seedValues: Map) { + MockPlatformParameterSingleton.seedPlatformParameterMap.putAll(seedValues) + ApplicationProvider.getApplicationContext().inject(this) +} + +@Module +class TestModule { + @Provides + fun provideMockPlatformParameterSingleton( + platformParameterSingletonImpl: PlatformParameterSingletonImpl + ) : PlatformParameterSingleton { + return MockPlatformParameterSingleton(platformParameterSingletonImpl) + } +} +``` + +Note : To understand the underlying mechanism of this test, you will need to understand how these platform parameters reach the dagger graph. You can refer to this [document](https://docs.google.com/document/d/1o8MtAO8e8bX7UtWFYx-T9G4vCGRfvY9oIwDutDn4pVM/edit#heading=h.m1q1hwhhqigf) for detailed explanation about the flow for the platform parameter architecture. diff --git a/wiki/README.md b/wiki/README.md new file mode 100644 index 00000000000..f3e4b194762 --- /dev/null +++ b/wiki/README.md @@ -0,0 +1 @@ +Have an idea for how to improve the wiki? Please help make our documentation better by following our [instructions for contributing to the wiki.](https://github.com/oppia/oppia-android/wiki/Wiki#contributing-to-the-wiki) diff --git a/wiki/RTL-Guidelines.md b/wiki/RTL-Guidelines.md new file mode 100644 index 00000000000..5e74e943213 --- /dev/null +++ b/wiki/RTL-Guidelines.md @@ -0,0 +1,48 @@ +# What is RTL? +The main difference between left-to-right (LTR) and right-to-left (RTL) language scripts is the direction in which content is displayed: + +* LTR languages display content from left to right +* RTL languages display content from right to left + +RTL content also affects the direction in which some icons and images are displayed, particularly those depicting a sequence of events. + +In general, the passage of time is depicted as left to right for LTR languages, and right to left for RTL languages. + +![](https://user-images.githubusercontent.com/53938155/145036934-691c6bda-a58b-4977-9247-cb6e3830dee7.png) + +When a UI is changed from LTR to RTL (or vice-versa), it’s often called mirroring. An RTL layout is the mirror image of an LTR layout, and it affects layout, text, and graphics. + +# What changes in RTL? + +When a UI is mirrored, these changes occur: + +* Text fields icons are displayed on the opposite side of a field +* Navigation buttons are displayed in reverse order +* Icons that communicate direction, like arrows, are mirrored +* Text (if it is translated to an RTL language) is aligned to the right + +These items are not mirrored: + +* Untranslated text (even if it’s part of a phrase) +* Icons that don’t communicate direction, such as a camera +* Numbers, such as those on a clock and phone numbers +* Charts and graphs + +Text should always be in the correct direction for the language it’s in. For example, any LTR words, such as a URL, will continue to be shown in an LTR format, even if the rest of the UI is in RTL. + +![](https://user-images.githubusercontent.com/53938155/145037261-2f7afb57-cbeb-4a6e-8ac7-c47261790945.png) + +# Testing app for RTL Layouts +To test any screen for RTL language follow these steps: +1. Enable [`Developer Options`](https://developer.android.com/studio/debug/dev-options) on your device. +2. Set language to `Arabic [XB]` or `cibarA [XB]` with the help of this [link](https://developer.android.com/guide/topics/resources/pseudolocales). + +Make this language as primary language from your `Language Preferences` in `Settings` and now open the oppia app. You will notice that all characters appear from right-to-left similar to what `Arabic` languages follow. +The screen will look something like this: + + + +# Reference Documentation +* [Oppia-Android RTL Issues](https://docs.google.com/document/d/1Fl1ar5vcdLvay7ZIJLUFQro1wEf1yUEicwF-CKcvwJ0/edit#) +* [RTL Support Milestone](https://github.com/oppia/oppia-android/milestone/40) +* [Guidelines for RTL](https://material.io/design/usability/bidirectionality.html) diff --git a/wiki/Revert-&-regression-policy.md b/wiki/Revert-&-regression-policy.md new file mode 100644 index 00000000000..ebaa813b506 --- /dev/null +++ b/wiki/Revert-&-regression-policy.md @@ -0,0 +1,35 @@ +When you find yourself in a situation where something seems newly broken in the app, it's important to: +1. Isolate the problem & [file an issue](https://github.com/oppia/oppia-android/issues/new?assignees=&labels=Type%3A+Improvement%2C+Status%3A+Not+started&template=feature_request.md&title=) for it +2. Identify the commit which introduced the regression (see below section) +3. [Revert the PR](https://github.blog/2014-06-24-introducing-the-revert-button/) associated with the commit & send it as a PR for review (the bug you filed in (1) should be marked as fixed by this PR) +4. Once the revert commit is submitted, reopen any issues fixed by the original PR that was reverted + +If you get stuck along the way, please mention in the issue that you filed in (1) that you are unable to locate commit introducing the regression. + +## Why do we revert PRs rather than just fix them? + +Leaving problems checked in can result in the following two situations: +1. As the known issues are fixed, others go unnoticed or are discovered later (potentially by users) +2. The original commit may become non-revertible without manual conflict resolution as more changes are checked in on top of that + +For this reason, the team's policy is to quickly revert regressions when they're found. This immediately unblocks team members and provides time for the original PR author to more carefully investigate the underlying issue & potential edge cases. + +## How to find the offending commit + +1. Always checkout an unchanged, up-to-date copy of develop & verify that the issue is still occurring. Note this commit hash. + - If you can't repro the issue, double check that it isn't flaky (try a few times). If you still can't, it's likely the issue has been resolved. Please still file an issue and leave notes for what you tried so that the team can triage accordingly. +2. Go back to an earlier commit on develop that doesn't repro this issue (if you're unsure, try something at the bottom of [this page](https://github.com/oppia/oppia-android/commits/develop) or earlier, if the regression was introduced a while ago). +3. Perform a binary search until you find the commit that introduced the issue. Specifically: + - Open up a text editor and list all of the commit hashes on develop between the one that you found is reproing the regression and the commit not reproing the issue + - Pick the middle hash of the list, check out that commit, and verify whether the issue regresses. If it does, this becomes your new lower bound. If it doesn't, this becomes your new upper bound. Like standard [binary search](https://en.wikipedia.org/wiki/Binary_search_algorithm), the number of commits you're considering is cut in half during each check. + +## What do I do if I caused a regression? + +If it hasn't been reverted, please follow the steps above to revert. Once your PR is reverted, follow these steps: +1. Ensure you can repro the issue fully. If you can't, talk to the original author to better understand how to repro the issue. +2. Make sure you fully understand _why_ your change caused the observed regression(s)--without this step, no progress can actually be made. Remember: fix issues not symptoms. We don't want the observed problem to just go away, we want the underlying root cause to be understood & addressed. +3. Once you understand the root cause, think through potential edge cases that may break other aspects of the app. It's quite likely that the observed regression was just one of several broken behaviors caused by the underlying issue. +4. Turn the behaviors from (3) into tests. Verify that these tests fail without a fix, and pass with a fix to the original regression. +5. Submit a new PR with your original changes + the fixes and tests from (4). Make sure to detail the investigation in this new PR, and reference the original issue that tracked the regression. + +Finally, be aware that everyone causes regressions. While we have a lot checks in place to try and avoid regressions from being checked in, they _will_ still happen. Focus on learning from the situation rather focusing on the fact that a regression happened. \ No newline at end of file diff --git a/wiki/SLoP-2020.md b/wiki/SLoP-2020.md new file mode 100644 index 00000000000..f7eab62b416 --- /dev/null +++ b/wiki/SLoP-2020.md @@ -0,0 +1,73 @@ +**_Note: For details about the programme have a look at [Oppia-SLoP](https://github.com/oppia/oppia/wiki/SLoP-2020)._** + +# Instructions for students +### How do I get started? +1. Register as a student on the SLoP [website](https://slop.dscdaiict.in/). +2. Follow the instructions on the [Getting Started page](https://github.com/oppia/oppia-android/wiki). **Please note, when filling up the contributor survey please indicate that you are taking part in SLoP.** +3. Once you have submitted the CLA and New Contributor Survey, you will be contacted by your on-boarding mentor (mostly within 24 hours) who will help you make your first contributions to Oppia-Android. But while you wait for the mentor to contact you, feel free to take a look at following resources: + - [Oppia-android codebase and architecture](https://github.com/oppia/oppia-android/wiki/Overview-of-the-Oppia-Android-codebase-and-architecture) + - [Guidance on submitting PR](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR) + - [Oppia-Android testing](https://github.com/oppia/oppia-android/wiki/Oppia-Android-Testing) + - [Working on UI](https://github.com/oppia/oppia-android/wiki/Working-on-UI) + - You can also have a look at the issues with the following labels: ”good first issue”, “good second issue”, “[SLoP 2020](https://github.com/oppia/oppia-android/issues?q=is%3Aopen+is%3Aissue+label%3A%22SLoP+2020%22+no%3Aassignee)” android to pick an issue that suits your skills and interests. +4. Your onboarding mentor will suggest starter issues for you to work on. These issues will have score labels of the form “**SLoP-\**”. +5. If you would like to take up issues that are not part of above labels or do not have a score label, please contact your onboarding mentor and request for the score for the issue. Your mentor will assign a score to the issue and communicate the same to the org admins. The org admins will then add the relevant label to the issue. + +### Expectations from students +1. Look for issues which are not assigned to anyone and comment on the issue mentioning your mentor name that you would like to work on it. Once the mentor approves you can start working on it. +2. Make sure that the issue that you are working on has a score assigned to it before you finish the PR for that issue. +3. Keep in touch with your mentor and give updates on regular time intervals. This can be something which you can figure out with your mentor. It's a good practice to communicate to your mentor atleast once a week. +4. Your PRs should get reviewed by your mentor within 24 hours but if for some reason the mentor is not reachable for more than 24 hours then assign your PRs to Rajat (@rt4914) for review. **NOTE: wait for atleast 24 hours to get a reply from a mentor.** +5. In case of any doubts or issues you should immediately talk to your mentor. + +# Instructions for mentors + +### Who are the mentors? +For initial 2 PRs default mentors will be Ben and Rajat. +After 2 PRs tentative mentor list: Akshay, Mohamed, Pulkit, Sarthak + +### Expectations +1. Please review assigned PRs within a 24 hr period. + - If you cannot review in that time frame, due to a one-off problem, please communicate the same in the PR thread and mention when you + - would be able to review the PR. +If you cannot review PRs in a 24 hr time frame for an extended period, consider temporarily assigning code-ownership to another experienced contributor. +2. If a student shows no activity for an assigned issue for >1 week, de-assign them so that other contributors get a chance to work on the issue. +3. Project leads may need to score project specific issues if a student requests to work on them (see “Scoring”). +4. Mentors are encouraged to have check-ins with their students on a weekly basis to make sure the students aren't stuck and are having a positive experience. +5. The level of mentoring is expected to be slightly higher than that of hacktoberfest and dealing with new contributors but less than that of GSoC. +Please note: Apart from the standard “24 hr review” rule, there is no formal time commitment for a mentor. + +### Scoring Policy +The set of points allowed to be allocated to each issue are: +* 5 Points - Level 1 +* 25 Points - Level 2 +* 75 Points - Level 3 +* 150 Points - Level 4 + +5 Points +* Very Easy Issue - Beginner level +* GitHub Issue Label: SLoP-5 +* Example: 1 line change in code, Typo fix in Documentation + +25 Points +* Easy-Moderate issue +* GitHub Issue Label: SLoP-25 +* Example: A small bug fix, minor refactor or contributions spanning to +few lines of code + +75 Points +* Moderately Difficult issue +* GitHub Issue Label: SLoP-75 +* Example: Addition of small feature, contribution that has fair bit of +learning involved + +150 Points +* Difficult Issue +* GitHub Issue Label: SLoP-150 +* Example: Major bug fix, a contribution that requires heavy effort and +research, Good new feature implemented + +NOTE: At any point before PR gets merged mentors can increase or decrease points. + +### Escalation Policy +If there are any SLoP-related doubts / concerns, please reach out to the org admins (@kevintab95, @rt4914, @BenHenning). diff --git a/wiki/Static-Analysis-Checks.md b/wiki/Static-Analysis-Checks.md new file mode 100644 index 00000000000..0a988854067 --- /dev/null +++ b/wiki/Static-Analysis-Checks.md @@ -0,0 +1,215 @@ +# Background +Static analysis is a method of debugging by examining source code before a program is run. It’s done by analyzing a set of code against a set (or multiple sets) of coding rules. + +The primary benefit is that it increases our ability to reliably enforce best practices. This is because humans make mistakes. Humans can unknowingly miss some places while reviewing, but with static analysis, this scenario will never happen. + +It takes time for developers to do manual code reviews. Automated tools are much faster. +Static code checking helps to detect problems early on by pinpointing exactly where the error is in the code before a reviewer even looks at it. + +
+ +

+Development flow without static analysis figure + +

Development flow without static analysis
+

+ +


+ +

+Development flow with static analysis figure + +

Development flow with static analysis
+

+ +
+ +We can see how a static analysis tool (in this case a linter) simplified the flow and saved the time of both the developer and the reviewer which was wasted earlier in correcting the lint errors. + +This page outlines the static analysis checks implemented in Oppia Android, instructions for how to utilize & maintain them, and instructions for introducing new checks to help enforce best practices as the codebase continues to scale. + +# Static analysis checks in Oppia Android + +## RegexPatternValidation Check + +### Generic regex pattern matching against file names +This check ensures that there are no prohibited file path patterns present in the repository. + +#### Purpose & adding new patterns +This check is needed so as to prevent a particular type of file from being added into a wrong directory. + +For example: If we want to prevent activities from being added into the any directory except testing and app, then we have to add its regex pattern in the [scripts/assets/filename_pattern_validation_checks.textproto](https://github.com/oppia/oppia-android/blob/2da95a53928bc989f5959fbac211f7f7ca0a753f/scripts/assets/filename_pattern_validation_checks.textproto) file like this: + +``` +filename_checks { + prohibited_filename_regex: "^((?!(app|testing)).)+/src/main/.+?Activity.kt" + failure_message: "Activities cannot be placed outside the app or testing module" +} +``` + +#### Fixing failures +In general, failures for this check should be fixed by moving the file to the correct directory. In cases where that can’t happen or the check is wrong, please: + +1. Add the file as an exemption in [scripts/assets/filename_pattern_validation_checks.textproto](https://github.com/oppia/oppia-android/blob/2da95a53928bc989f5959fbac211f7f7ca0a753f/scripts/assets/filename_pattern_validation_checks.textproto) for the corresponding failing check in , e.g.: +``` +filename_checks { + prohibited_filename_regex: "^((?!(app|testing)).)+/src/main/.+?Activity.kt" + failure_message: "Activities cannot be placed outside the app or testing module" + exempted_file_name: "testing/src/main/SampleActivity.kt" +} +``` +2. Add an explanation to your PR description detailing why this exemption is correct + +### Generic regex pattern matching against file contents +This check ensures that there are no prohibited file contents present in the repository. + +#### Purpose & adding new patterns +This check is needed so as to prevent the use of any prohibited content in a file which allows the team to introduce specific best practices checks. + +For example: If we want to prevent the use of support library in the repository, then we have to add its regex pattern in the [scripts/assets/file_content_validation_checks.textproto](https://github.com/oppia/oppia-android/blob/2da95a53928bc989f5959fbac211f7f7ca0a753f/scripts/assets/file_content_validation_checks.textproto) file like this: + +``` +file_content_checks { + filename_regex: ".+?.kt" + prohibited_content_regex: "^import .+?support.+?$" + failure_message: "AndroidX should be used instead of the support library" +} +``` + +#### Fixing failures +In general, failures for this check should be fixed by not using the prohibited content in the repository (the error message of the failure should explain what should be used, instead). + +In cases where that can’t happen or the check is wrong, please: + +1. Add the file as an exemption in [scripts/assets/file_content_validation_checks.textproto](https://github.com/oppia/oppia-android/blob/2da95a53928bc989f5959fbac211f7f7ca0a753f/scripts/assets/file_content_validation_checks.textproto) for the particular check which is failing. +For example: a file which has relative path to root as app/src/main/java/org/oppia/android/home/SampleActivity.kt, should be added as follows to the corresponding failing check: +``` +file_content_checks { + filename_regex: ".+?.kt" + prohibited_content_regex: "^import .+?support.+?$" + failure_message: "AndroidX should be used instead of the support library" + exempted_file_name: "app/src/main/java/org/oppia/android/home/SampleActivity.kt" +} +``` +2. Add an explanation to your PR description detailing why this exemption is correct + +## XML syntax check +This check ensures that all the XML files present in the repository are syntactically correct. + +### Purpose +There are no linters to evaluate XML correctness. This check performs a fast check of correctness before any attempts to build the app will complete, allowing for a faster feedback cycle for potential XML issues. + +### Fixing failures +To fix failures for this check: correct the syntax of the XML file which is failing the check. + +## Test file presence check +This check ensures that all the production (file which is not a test file) Kotlin files must have a corresponding test file present. + +### Purpose +To ensure that we are not missing tests for any production file being added to the codebase, this check is needed. It helps us to ensure production files have corresponding tests and reminds the contributor to add tests for new code added. + +### Fixing failures +In general, failures for this check should be fixed by: adding a corresponding test file which has tests for the file which fails this check. Note that: the file name of the added test file must be file_name without extension + “Test.kt”. +For example: For a file named as “SampleFragment.kt” failing this check, the name of the added test file should be “SampleFragmentTest.kt”. + +In cases where a test can’t be added, or the check is wrong, please: + +1. Add it as an exemption by providing its relative path to root in [script/assets/test_file_exemptions.textproto](https://github.com/oppia/oppia-android/blob/2da95a53928bc989f5959fbac211f7f7ca0a753f/scripts/assets/test_file_exemptions.textproto). +Also, note that the file paths in the textproto file are maintained in lexicographical order. While adding any new file, please add it only at the correct lexicographical position, so that the list remains sorted. For example if we want to add the 'ActivityComponent.kt' file to the exemption list, at the correct lexicographical position in the textproto file add: +``` +exempted_file_path: "app/src/main/java/org/oppia/android/app/activity/ActivityComponent.kt" +``` +2. Add an explanation to your PR description detailing why this exemption is correct. + +Following are the cases where its valid to have test file exemptions: +1. Interface files +2. Files with only constants defined (no logic) + +## Accessibility label check +This check ensures that activities are defined with accessibility labels. + +### Purpose +For users requiring accessibility assistance (e.g. using a screen reader), activity labels are very important since they are read by screen readers to provide context on where the user is within the app. Suppose, on the button click, the app transits to a new activity. An unassisted user will likely face no barriers in understanding that a new screen/activity has opened, but accessibility users need an indication so that they can be notified that the transition to a new activity has been successfully made. For this reason, all activities must be defined with accessibility labels. + +### Fixing failures +If it’s a production activity, add a label. + +If it’s a test activity or an activity that can’t/shouldn’t have a label, add an exemption by providing its relative path to root in [script/assets/accessibility_label_exemptions.textproto](https://github.com/oppia/oppia-android/blob/2da95a53928bc989f5959fbac211f7f7ca0a753f/scripts/assets/accessibility_label_exemptions.textproto). If an exemption is added, please include a rationale in your PR description explaining why this exemption is correct. +Also, note that the exemptions in the textproto file are maintained in lexicographical order. While adding any new Activity, please add it only at the correct lexicographical position, so that the list remains sorted. + +For example if we want to add the 'RecentlyPlayedActivity' to the exemption list, add: +``` +exempted_activity: "app/src/main/java/org/oppia/android/app/home/HomeActivity" +``` +at the correct lexicographical position in the textproto file. + +## KDoc validity check +This check ensures that all non-private declarations/members are documented with KDocs. + +### Purpose +To ensure that we are not missing the documentation for any non-private decorations or members, this check is needed. Documentation is very important as it helps team members understand specific contexts within the codebase. Therefore, we should document as much as we can. Further, to ensure consistency we generally expect that all non-private members within Kotlin files are documented (per our [style guide](https://developer.android.com/kotlin/style-guide#usage)). + +### Fixing failures +To fix failures for this check is to add a KDoc at the member which is missing it. Please refer to the failure logs to get the exact location (file path and line number) of the failure. For guidance on how to write KDocs, please look at our [style guide](https://developer.android.com/kotlin/style-guide). + +## TODO open checks +This check ensures that every TODO present in the codebase corresponds to open issues on GitHub and is also correctly formatted as per the convention. + +### Purpose +To avoid scenarios where a TODO was added not corresponding to an open issue on GitHub, this check is particularly needed. Having a corresponding issue for a TODO helps us to track its progress. Also, in order to maintain consistency we want all the TODOs to be formatted as per the convention. + +### Fixing failures + +#### TODO formatting failure +To fix the formatting failure, please make sure that the added TODO is strictly as per this format (please pay attention to the whitespaces): + +Kotlin & Java files: +``` +// TODO(#ISSUE_NUMBER): +``` +Shell & BUILD/Bazel files: +``` +# TODO(#ISSUE_NUMBER): +``` +XML files: +``` + +``` +#### TODO Open issue failure +To fix this failure: there are 3 ways: +1. Repurpose the TODO to a new issue. +2. If the TODO has been resolved then please remove it from the repository. +3. Reopen the issue. +#### Case when using ‘TODO’ keyword for documentation purposes +If it’s a case where a ‘TODO’ keyword has been used for documentation purposes or if it's not meant to correspond to a future work, then please add an exemption for it. Add a new TODO exemption in the [scripts/assets/todo_open_exemptions.textproto](https://github.com/oppia/oppia-android/blob/2da95a53928bc989f5959fbac211f7f7ca0a753f/scripts/assets/todo_open_exemptions.textproto). +Example: +``` +todo_open_exemption { + exempted_file_path: , + line_number: +} +``` + +## TODO issue resolved check +The check ensures that a TODO issue is not closed until all of its corresponding TODO items are resolved. + +### Purpose +We need this check to avoid scenarios like missing a TODO item not being resolved and its issue being mistakenly closed, since this should result in confusing contexts where a team member looking up a particular TODO from code sees that the issue is closed and isn’t sure whether the issue has actually been resolved. If a TODO issue is closed without resolving all of its TODO items, then it will be reopened automatically by the GitHub actions bot with a comment containing the unresolved TODOs. + +### Fixing failures +To fix the failures for this check: resolve the TODO items and then close the issue. If the TODO items are not resolved the issue will remain in the open state. + +## Java lint check +// TODO([#3690](https://github.com/oppia/oppia-android/issues/3690)): Complete static checks Wiki + +## Kotlin lint check +// TODO([#3690](https://github.com/oppia/oppia-android/issues/3690)): Complete static checks Wiki + +## Protobuf lint check +// TODO([#3690](https://github.com/oppia/oppia-android/issues/3690)): Complete static checks Wiki + +## Bazel lint check +// TODO([#3690](https://github.com/oppia/oppia-android/issues/3690)): Complete static checks Wiki + +# How to add a new Static analysis check +// TODO([#3690](https://github.com/oppia/oppia-android/issues/3690)): Complete static checks Wiki diff --git a/wiki/Teams-at-Oppia-Android.md b/wiki/Teams-at-Oppia-Android.md new file mode 100644 index 00000000000..726504e26d8 --- /dev/null +++ b/wiki/Teams-at-Oppia-Android.md @@ -0,0 +1,44 @@ +The Oppia team is a distributed team of contributors from all over the world. To ensure that the project is as stable as possible, we have several infrastructure teams devoted to maintaining the health of various aspects of the development workflow. We also have an onboarding team that helps new contributors get started with the project and a welfare team responsible for assisting new contributors. + +This wiki page explains the different teams in Oppia and their composition. + +### CLaM Team + +CLaM team is responsible for most of the UI based code (app-layer), which includes exploration player, question player, concept-card, subtopics, topics, etc. All these user-facing features must be RTL supportive, accessible by all and should work on all devices (mobile + tablet). + +**Team contacts:** Ben Henning (@BenHenning) (lead), Adhiambo Peres (@adhiamboperes) + +### Dev-workflow Team +The dev-workflow team ensures that the Oppia development process is smooth, stable and enjoyable, by ensuring that the following always hold: + +1. There are no issues with the codebase setup (especially for new contributors). +2. Automated checks work as intended and are not unduly burdensome on both contributor’s machines and CI servers. +3. The technical documentation on the wiki is well-arranged, useful, and correct. +4. There are no security issues relating to npm dependencies. +5. The review process is speedy and streamlined. + +Long-term projects include: + +1. Working with the Onboarding team to identify areas where new contributors get stuck during the onboarding process and taking steps to fix those issues. +2. Streamlining the code review flow by: + * adding pre-submit checks for common errors + * enabling Oppiabot to automatically handle review/code-owner assignments + * speeding up the CI processes. + +**Team contact:** Ben Henning (@BenHenning) (lead) + +### Infrastructure Teams + +##### Release Process Team +This team is responsible for ensuring that Oppia releases happen smoothly, correctly, and on time. Long-term projects include: +1. Streamlining the release process, and automating as many parts as possible, in order to reduce the chance of human error. +2. Adding automatic safeguards to ensure the correctness of releases. +3. Organizing the release coordinator rotation. + +**Team Contact:** Ben Henning (@BenHenning) + + +### Onboarding/Welfare Team +This team is a group of Oppia developers who are committed to helping developers to be able to unblock themselves when they face any problems. They also aim to welcome new contributors and answer their questions. + +**Team contact:** Mohit Gupta (@MohitGupta121) \ No newline at end of file diff --git a/wiki/Troubleshooting-Installation.md b/wiki/Troubleshooting-Installation.md new file mode 100644 index 00000000000..7ae884ba62f --- /dev/null +++ b/wiki/Troubleshooting-Installation.md @@ -0,0 +1,66 @@ +Here are some general troubleshooting tips for oppia-android. The specific platforms are Linux, Windows and Mac OS. + +### General issues + +1. If you find any error related to `cURL`, please set up cURL on your machine. For Linux, you can use `sudo apt install curl`. No need to set up `cURL` for Windows as you are using git bash command line.
+ +2. If you find any error which says `java: command not found`, please check you have Java installed correctly in your machine and the [environment path variable](https://www.java.com/en/download/help/path.html) is also set up correctly. + +3. If you find any error related to Kotlin or Java/Checkstyle while pushing the code, please check [this link](https://github.com/oppia/oppia-android/wiki/Android-Studio-UI-based-Github-workflow#how-to-fix-push-failures). + +4. If you see the error + + ``` + Could not resolve protoc-3.8.0-osx-aarch_64.exe (`com.google.protobuf:protoc:3.8.0`) + ``` + + then please follow the 2nd step mentioned in [this wiki](https://github.com/oppia/oppia-android/wiki/Installing-Oppia-Android#install-oppia-android) for Mac with Apple silicon(M1/M2) chips. + +5. If you see the error + + ``` + Deprecated Gradle features were used in this build, making it incompatible with Gradle 7.0. + ``` + + then it's fine to ignore it. The message just appears to be a warning. We don't use Gradle 7.0, so this warning is fine to ignore. + +6. If you see the error + + ``` + Error `Class 'org.oppia.android.app.profile.PinPasswordActivityTest' not found in module 'oppia-android.app'` + ``` + + or `Module not specified` while running Unit Tests, try to downgrade Android Studio to [Bumblebee (Patch 3)](https://developer.android.com/studio/archive). That should resolve this issue. + + +### Bazel issues + +1. No matching toolchains (sdk_toolchain_type) + ``` + ERROR: While resolving toolchains for target //:oppia: no matching toolchains found for types + @bazel_tools//tools/android:sdk_toolchain_type + ERROR: Analysis of target '//:oppia' failed; build aborted: no matching toolchains found for types + @bazel_tools//tools/android:sdk_toolchain_type + INFO: Elapsed time: 12.805s + INFO: 0 processes. + FAILED: Build did NOT complete successfully (13 packages loaded, 51 targets configured) + ``` + [Steps](https://docs.bazel.build/versions/main/tutorial/android-app.html#integrate-with-the-android-sdk) to add ANDROID_HOME environment variable. + +2. If you encounter the following: + + ``` + ERROR: While parsing option --override_repository=android_tools=~/oppia-bazel/android_tools: Repository + override directory must be an absolute path + ``` + + Try to delete the `.bazelrc` file to solve the above error. + +3. **java.lang.ClassNotFoundException: com.android.tools.r8.compatdx.CompatDx** + + If, when building the app binary, you encounter a failure that indicates that the CompatDx file cannot be found, this is likely due to you using a newer version of the Android build tools. You can manually downgrade to an older version of build-tools (particularly 29.0.2). Unfortunately, this can't be done through Android Studio but it can be done over a terminal. Follow the instructions listed [here](https://github.com/oppia/oppia-android/issues/3024#issuecomment-884513455) to downgrade your build tools & then try to build the app again. + + +### Can’t find a particular issue? + +If the error you get is not in the Troubleshooting section above, please create an issue providing all the necessary information and assign it to **@MohitGupta121**. diff --git a/wiki/Updating-Maven-Dependencies.md b/wiki/Updating-Maven-Dependencies.md new file mode 100644 index 00000000000..cb08415d296 --- /dev/null +++ b/wiki/Updating-Maven-Dependencies.md @@ -0,0 +1,140 @@ +All the third-party Maven dependencies used in Oppia-Android along with their versions are mentioned in the [versions.bzl](https://github.com/oppia/oppia-android/blob/develop/third_party/versions.bzl) file that resides in the `third_party` package. To add/delete/update any dependency in `MAVEN_PRODUCTION_DEPENDENCY_VERSIONS` or `MAVEN_TEST_DEPENDENCY_VERSIONS` dictionaries, please follow the below steps. + +## Updating `maven_install.json` + +1. Ensure that after making changes in the list of dependencies, the final list is always lexicographically sorted. +2. After updating the dependencies, run the following command. +``` +REPIN=1 bazel run @unpinned_maven//:pin +``` + +## Updating `maven_dependencies.textproto` +You will also need to run the [GenerateMavenDependenciesList.kt](https://github.com/oppia/oppia-android/blob/develop/scripts/src/java/org/oppia/android/scripts/maven/GenerateMavenDependenciesList.kt) script to update the [maven_dependencies.textproto](https://github.com/oppia/oppia-android/blob/develop/scripts/assets/maven_dependencies.textproto) file. This text proto file contains the license links for all the maven third-party dependencies on which Oppia Android depends. Please make sure that before running the script, you have successfully updated [maven_install.json](https://github.com/oppia/oppia-android/blob/develop/third_party/maven_install.json) by following the above-mentioned [guide](https://github.com/oppia/oppia-android/wiki/Updating-Maven-Dependencies#updating-maven_installjson). +To run this script, run the following commands. + +``` +cd ~/opensource/oppia-android +``` +The above command ensures that the terminal points to the root directory `oppia-android` repository. Note that if you have configured a different path to the `oppia-android` repository then you should modify the above command accordingly ( `cd ~/` ). + +#### Running `GenerateMavenDependenciesList.kt` script +After the terminal points to the Oppia-android repository, run the bazel run command to execute the Kotlin script. +``` +bazel run //scripts:generate_maven_dependencies_list -- $(pwd) third_party/maven_install.json scripts/assets/maven_dependencies.textproto scripts/assets/maven_dependencies.pb +``` + +## Handling Exception: `Too few arguments passed` +If after running the script the exception message says: **Too few arguments passed**, then please ensure that you copied the command correctly from [here](https://github.com/oppia/oppia-android/wiki/Updating-Maven-Dependencies#running-generatemavendependencieslistkt-script). +The script accepts 4 parameters to be passed to run successfully: +1. **_path_to_directory_root_**: directory path to the root of the Oppia Android repository, e.g - `home//opensource/oppia-android` +2. **_path_to_maven_install_json_**: relative path to the maven_install.json file, e.g - `third_party/maven_install.json` +3. **_path_to_maven_dependencies_textproto_**: relative path to the maven_dependencies.textproto, e.g - `scripts/assets/maven_dependencies.textproto` +4. **_path_to_maven_dependencies_pb_**: relative path to the maven_dependencies.pb file, e.g - `scripts/assets/maven_dependencies.pb` + + +## Handling Exception: `Licenses details are not completed` +The script can take about a minute to execute, and if the script fails with the exception: `Licenses details are not completed`, you will need to do some manual work in `maven_dependencies.textproto`. +The script would call out specific dependencies that need to be updated manually, e.g - + +``` +Please verify the license link(s) for the following license(s) manually in +maven_dependencies.textproto, note that only the first dependency that contains the license +needs to be updated and also re-run the script to update the license details at all places: + +license_name: Android Software Development Kit License +original_link: https://developer.android.com/studio/terms.html +verified_link_case: VERIFIEDLINK_NOT_SET +is_original_link_invalid: false +The first dependency that should be updated with the license: com.google.firebase:firebase-analytics:17.5.0 + +``` + +Go to `maven_dependencies.textproto` and find the dependency that is mentioned as `The first dependency that should be updated with the license` in the output. For example, in the above case, look for `com.google.firebase:firebase-analytics:17.5.0` in maven_dependencies.textproto and open the `original_link` of its license in your browser and check if the link points to any valid license or not. If the link does not point to a valid license, set the 'is_original_link_invalid' field of the license to 'true'. +For example, if the original_link https://www.example.com is invalid, then set `is_original_link_invalid` to true. + +``` +maven_dependency { + artifact_name: "artifact:name" + artifact_version: "1.0" + license { + license_name: "XYZ License" + original_link: "https://www.xyz.com" + is_original_link_invalid: true + } +} +``` + +### Categorizing the license link + +If the link does point to a valid license then choose the most appropriate category for the link: +1. scrapable_link: If the license text is plain text and the URL mentioned can be scraped directly from the original_link of the license. + e.g - https://www.apache.org/licenses/LICENSE-2.0.txt +2. extracted_copy_link: If the license text is plain text but can not be scraped directly from the original_link of the license. + e.g - https://www.opensource.org/licenses/bsd-license +3. direct_link_only: If the license text is not plain text, it's best to display only the link of the license. + e.g - https://developer.android.com/studio/terms.html + +After identifying the category of the license, modify the license to include one of the above-mentioned 'url'. e.g - +``` +license { + license_name: "The Apache Software License, Version 2.0" + original_link: "https://www.apache.org/licenses/LICENSE-2.0.txt" + scrapable_link { + url: "https://www.apache.org/licenses/LICENSE-2.0.txt" + } +} +``` + +Also, if the license falls in the `extracted_copy_link` category, then go to [Oppia-android-licenses](https://github.com/oppia/oppia-android-licenses) and find if there exists a copy of the license already in the repository. If there exists a copy of the license, perform the following steps to get the link for the license that can be scraped easily. +1. Click on the appropriate license file. +2. Now click on the raw button positioned in the left of the edit and delete button. +3. Copy the URL from the browser and mention it at the appropriate place. + +If the license does not exist in the Oppia-android-licenses repository, then coordinate with the Oppia Android team to fix it. Then repeat the above steps to update maven_dependencies.textproto. + + + + +After modifying `maven_dependencies.textproto` for all the called out licenses in the console output, re-run the script and see if any other error occurs. + +## Handling Exception: `License links are invalid or not available for some dependencies` + +If the script throws `License links are invalid or not available for some dependencies` exception, then the output would look something like this: + +``` +Please remove all the invalid links (if any) from maven_dependencies.textproto for the below mentioned dependencies and provide the valid license links manually. + +maven_dependency { + artifact_name: "io.fabric.sdk.android:fabric:1.4.7" + artifact_version: "1.4.7" + license { + license_name: "Terms of Service for Firebase Services" + original_link: "https://fabric.io/terms" + is_original_link_invalid: true + } +} + +maven_dependency { + artifact_name: "com.google.guava:failureaccess:1.0.1" + artifact_version: "1.0.1" +} +``` + +To fix the error, consider the above example. For the first maven_dependency: "io.fabric.sdk.android:fabric:1.4.7", the original_link is invalid, and hence we need to find a valid link for this dependency. Please coordinate with the Oppia Android team and find a valid link for this dependency. Once you have a valid link for this license then categorize it as mentioned [here](https://github.com/oppia/oppia-android/wiki/Updating-Maven-Dependencies#categorizing-the-license-link). + +For the second maven_dependency: "com.google.guava:failureaccess:1.0.1", you need to find a license by coordinating with the Oppia Android team and then specify it under the artifact_version field of the dependency. e.g - + +``` +maven_dependency { + artifact_name: "com.google.guava:failureaccess:1.0.1" + artifact_version: "1.0.1" + license { + license_name: "The Apache Software License, Version 2.0" + scrapable_link { + url: "https://www.apache.org/licenses/LICENSE-2.0.txt" + } + } +} +``` + +After updating maven_dependencies.textproto for all the called out dependencies, re-run the script. The script would pass if all the dependencies are updated successfully, and if it doesn't identify the exception being thrown and try to fix it with the help of the above-mentioned details. \ No newline at end of file diff --git a/wiki/Wiki.md b/wiki/Wiki.md new file mode 100644 index 00000000000..0e45c595b0a --- /dev/null +++ b/wiki/Wiki.md @@ -0,0 +1,82 @@ +## Table of contents + +* [Contributing to the wiki](#contributing-to-the-wiki) + * [Opening a pull request](#opening-a-pull-request) + * [Failed to push changes to wiki upon PR merge.](#failed-to-push-changes-to-wiki-upon-pr-merge) +* [Implementation details](#implementation-details) + * [Wiki architecture](#wiki-architecture) + * [When you make changes through the web interface](#when-you-make-changes-through-the-web-interface) + + +## Contributing to the wiki + +If you notice something about the wiki that could be improved, please let us know! There are a couple of ways you can do so: + +* If the improvement you have in mind is big, you want feedback before you start working on it, or you don't have time to make the change yourself, [open an issue](https://github.com/oppia/oppia-android/issues/new) in the oppia-android repository. +* If you can make the change yourself, see the instructions below for opening a pull request (PR). + +### Opening a pull request + +For your *first contribution* to oppia-android wiki, you'll need to set up your repository (you only have to do this once). Please follow the instructions in the [Installing Oppia Android](https://github.com/oppia/oppia-android/wiki/Installing-Oppia-Android) to set up your repository. + +Then for every new contribution (including your first), you should follow these steps: + +1. Checkout the `develop` branch and pull in the latest changes from upstream: + + ```console + git checkout develop + git pull upstream develop + ``` + +2. Create a new branch for your changes: + + ```console + git checkout -b {{branch name}} + ``` + +3. Make your changes, being sure to follow our [style guide](https://github.com/oppia/oppia/wiki/Wiki-style-guide). You can use whatever text editor you prefer for this. + +4. Commit your changes. You can make multiple commits as you write if you prefer. + + ```console + git add {{paths to the files you changed}} + git commit + ``` + +5. Push your changes to your fork (called `origin` by default): + + ```console + git push -u origin {{branch name}} + ``` + +6. [Open a pull request.](https://github.com/oppia/oppia-android/compare) Remember to click the "compare across forks" link since your changes are on a different fork than oppia-android. The base for your PR should be the `develop` branch on the `oppia/oppia-android` repository. + +7. Wait for the reviewers to review your PR. + +8. Once the reviewers leaves comments, respond to them and make changes as needed. Please do not resolve review threads--let the reviewer do that. Repeat as needed until reviewers approve. + +9. Once reviewers have approved, they will merge your PR, and your changes will be automatically deployed to the Oppia wiki. Congratulations! :tada: + +### Failed to push changes to wiki upon PR merge. + +If the deployment of changes to the wiki following the merging of a pull request was unsuccessful, you can re-run the failed "Deploy to wiki" workflow to deploy to the wiki by following these steps: + +1. Navigate to the Oppia Android wiki repository's Actions tab. +2. Select the "Deploy to wiki" workflow. +3. Refer to these [instructions](https://docs.github.com/en/actions/managing-workflow-runs/re-running-workflows-and-jobs#re-running-all-the-jobs-in-a-workflow) to re-run the workflow. + +## Implementation details + +### Wiki architecture + +Our wiki consists of the following components: + +* The `oppia/oppia-android.wiki` git repository automatically created by GitHub to hold the wiki viewable at https://github.com/oppia/oppia-android/wiki. This is our deployment repository where we put wiki source files to be viewed by the community. +* The `/wiki` folder in the `oppia/oppia-android` repository is our source repository where we store and edit the wiki source files. We consider this to be the single source of truth for our wiki. +* A [`wiki.yml`](https://github.com/oppia/oppia-android/blob/develop/.github/workflows/wiki.yml) workflow in oppia-android deploys any new commits in oppia-android to the deployment repository. The workflow is activated whenever a pull request that modifies any file in the `/wiki` directory is pushed to the `develop` branch. + +### When you make changes through the web interface + +If you change the wiki through the web interface at https://github.com/oppia/oppia-android/wiki, the [`wiki.yml`](https://github.com/oppia/oppia-android/blob/develop/.github/workflows/wiki.yml) workflow will be triggered and the wiki will be reset to by pushing the files from the `/wiki` folder to the `oppia/oppia-android.wiki` git repository. +> **Note** +> The `/wiki` folder in the `oppia/oppia-android` repository is the single source of truth for our wiki. diff --git a/wiki/Work-Manager.md b/wiki/Work-Manager.md new file mode 100644 index 00000000000..5cd813addb1 --- /dev/null +++ b/wiki/Work-Manager.md @@ -0,0 +1,164 @@ +# What is WorkManager? +WorkManager is part of Android Jetpack and an Architecture Component for background work that needs a combination of opportunistic and guaranteed execution. Opportunistic execution means that WorkManager will do your background work as soon as it can. Guaranteed execution means that WorkManager will take care of the logic to start your work under a variety of situations, even if you navigate away from your app. + +WorkManager is an incredibly flexible library that has many additional benefits. These include: +- Support for both asynchronous one-off and periodic tasks +- Support for constraints such as network conditions, storage space, and charging status +- Chaining of complex work requests, including running work in parallel +- Output from one work request used as input for the next +- Handling API level compatibility back to API level 14 (see note) +- Working with or without Google Play services +- Following system health best practices +- LiveData support to easily display work request state in UI + +The WorkManager library is a good choice for tasks that are useful to complete, even if the user navigates away from the particular screen or your app. Some examples of tasks that are a good use of WorkManager: +- Uploading logs +- Applying filters to images and saving the image +- Periodically syncing local data with the network + +WorkManager offers guaranteed execution, and not all tasks require that. As such, it is not a catch-all for running every task off of the main thread. + +# Its Usage in Oppia Android +There are a few WorkManager classes you need to know about: + +- `Worker`: This is where you put the code for the actual work you want to perform in the background. You'll extend this class and override the doWork() method. +- `WorkRequest`: This represents a request to do some work. You'll pass in your Worker as part of creating your WorkRequest. When making the WorkRequest you can also specify things like Constraints on when the Worker should run. There are two types of work supported by WorkManager: OneTimeWorkRequest and PeriodicWorkRequest. +- `WorkManager`: This class actually schedules your WorkRequest and makes it run. It schedules WorkRequests in a way that spreads out the load on system resources, while honoring the constraints you specify. + + +In Oppia we are using WorkManager in two scenarios : +- To upload cached Logs (for Analytics) over FirebaseAnalytics whenever data connection and battery requirements are met. This was implemented by @Sarthak2601 during GSoC'20, for more details you can go through the [proposal idea](https://github.com/oppia/oppia/wiki/pdfs/GSoC2020SarthakAgarwal.pdf) +- To sync up the PlatformParameters from OppiaBackend whenever the app starts and the data + battery requirements are met. This was implemented by @ARJUPTA during GSoC'21, for more details you can go through the [proposal idea](https://github.com/oppia/oppia/wiki/pdfs/GSoC2021ArjunGupta.pdf) + +# How to use WorkManager +If you want to introduce a new feature or any change to the existing WorkManager implementation in oppia-android, here is the basic structure of files you need to keep in mind : + +1. Start with creating a Worker class (we have used `ListenableWorker` till now everywhere) for eg - MyFeatureWorker. + + ``` + class LogUploadWorker private constructor( + context: Context, + params: WorkerParameters, + ... + @BackgroundDispatcher private val backgroundDispatcher: CoroutineDispatcher + ) : ListenableWorker(context, params) { + @ExperimentalCoroutinesApi + override fun startWork(): ListenableFuture { + val backgroundScope = CoroutineScope(backgroundDispatcher) + val result = backgroundScope.async {...} + return if(checkWorkDone(result)) Result.success() else Result.failure() + } + } + ``` + +2. Then after implementing all the functionality in MyFeatureWorker, create a custom WorkerFactory class (for eg- MyFeatureWorkerFactory) so that we can provide any extra parameters if needed. + + ``` + class LogUploadWorkerFactory @Inject constructor( + private val workerFactory: LogUploadWorker.Factory + ) : WorkerFactory() { + + /** Returns a new [LogUploadWorker] for the given context and parameters. */ + override fun createWorker( + appContext: Context, + workerClassName: String, + workerParameters: WorkerParameters + ): ListenableWorker? { + return workerFactory.create(appContext, workerParameters) + } + } + ``` + +3. Provide an instance of this WorkerFactory class in the `WorkManagerConfigurationModule` so that a singular work manager configuration can be made for the entire app. + + ``` + @Module + class WorkManagerConfigurationModule { + + @Singleton + @Provides + fun provideWorkManagerConfiguration( + logUploadWorkerFactory: LogUploadWorkerFactory, + platformParameterSyncUpWorkerFactory: PlatformParameterSyncUpWorkerFactory + ): Configuration { + val delegatingWorkerFactory = DelegatingWorkerFactory() + delegatingWorkerFactory.addFactory(logUploadWorkerFactory) + delegatingWorkerFactory.addFactory(platformParameterSyncUpWorkerFactory) + return Configuration.Builder().setWorkerFactory(delegatingWorkerFactory).build() + } + } + ``` + +4. After all these steps create an Initializer class (for eg- MyFeatureWorkerInitializer) that will prepare and enqueue a WorkRequest for you at the time when app starts. + + ``` + @Singleton + class LogUploadWorkManagerInitializer @Inject constructor( + private val context: Context, + private val logUploader: LogUploader + ) : ApplicationStartupListener { + override fun onCreate() { + val workManager = WorkManager.getInstance(context) + logUploader.enqueueWorkRequestForEvents(workManager, workRequestForUploadingEvents) + logUploader.enqueueWorkRequestForExceptions(workManager, workRequestForUploadingExceptions) + } + } + ``` + +**Note** - All the parts of WorkManager implementation entirely lie in the domain layer, but there are few functionalities that you may need to acquire from other layers for eg- if you need to make a network request you would probably need to interact with data layer also. + +# Writing tests with WorkManager +For writing any test with WorkManager you will need to interact with +- *WorkManagerTestInitHelper* so that you can emulate the enquing and running of WorkRequests. + + ``` + @Before + fun setUp() { + setUpTestApplicationComponent() + context = InstrumentationRegistry.getInstrumentation().targetContext + val config = Configuration.Builder() + .setExecutor(SynchronousExecutor()) + .setWorkerFactory(logUploadWorkerFactory) + .build() + WorkManagerTestInitHelper.initializeTestWorkManager(context, config) + } + ``` +- *TestCoroutinesDispatcher* so that you can block the code execution up untill WorkRequest(s) are running. (ie. working with suspend functions) +- You might also need to introduce some fakes so that you can make sure the entities (object, classes, varaibles & constants etc.) over which you MyFeatureWorker depends doesn't have any bugs. + +Here is an exemplar test that is using WorkManager to enqueue a WorkRequest with any inputData (if needed). After we enqueue a request, the next step is to wait until its execution is completed and for that we are using testCoroutineDispatchers - +``` + @Test + fun testWorker_logEvent_withoutNetwork_enqueueRequest_verifySuccess() { + networkConnectionUtil.setCurrentConnectionStatus(NONE) + analyticsController.logTransitionEvent( + eventLogTopicContext.timestamp, + eventLogTopicContext.actionName, + oppiaLogger.createTopicContext(TEST_TOPIC_ID) + ) + + val workManager = WorkManager.getInstance(ApplicationProvider.getApplicationContext()) + + val inputData = Data.Builder().putString( + LogUploadWorker.WORKER_CASE_KEY, + LogUploadWorker.EVENT_WORKER + ).build() + + val request: OneTimeWorkRequest = OneTimeWorkRequestBuilder() + .setInputData(inputData) + .build() + + workManager.enqueue(request) + testCoroutineDispatchers.runCurrent() + val workInfo = workManager.getWorkInfoById(request.id) + + assertThat(workInfo.get().state).isEqualTo(WorkInfo.State.SUCCEEDED) + assertThat(fakeEventLogger.getMostRecentEvent()).isEqualTo(eventLogTopicContext) + } +``` + +In Oppia we write tests for both the Worker and its Initializer class. You can take a reference for the same from these files: + +Worker Tests - *[PlatformParameterSyncUpWorkerTest](https://github.com/oppia/oppia-android/blob/develop/domain/src/test/java/org/oppia/android/domain/platformparameter/syncup/PlatformParameterSyncUpWorkerTest.kt) OR [LogUploadWorkerTest](https://github.com/oppia/oppia-android/blob/develop/domain/src/test/java/org/oppia/android/domain/oppialogger/loguploader/LogUploadWorkerTest.kt)* + +Initializer Tests - *[PlatformParameterSyncUpWorkManagerInitializerTest](https://github.com/oppia/oppia-android/blob/develop/domain/src/test/java/org/oppia/android/domain/platformparameter/syncup/PlatformParameterSyncUpWorkManagerInitializerTest.kt) OR [LogUploadWorkManagerInitializerTest](https://github.com/oppia/oppia-android/blob/develop/domain/src/test/java/org/oppia/android/domain/oppialogger/loguploader/LogUploadWorkManagerInitializerTest.kt)* \ No newline at end of file diff --git a/wiki/Working-on-UI.md b/wiki/Working-on-UI.md new file mode 100644 index 00000000000..25ce06a8b8d --- /dev/null +++ b/wiki/Working-on-UI.md @@ -0,0 +1,73 @@ +Oppia-Android app mocks have been prepared on AdobeXD and the mocks have a version associated with them which is displayed on the bottom right corner of the "SplashScreen" page in mocks. Before working on any UI issue make sure that you have latest link of the mock associated with that UI. + +- [Mobile Portrait](https://xd.adobe.com/view/3dca36c2-5115-419c-b25e-0f10526b077c-6899/grid/) +- [Mobile Landscape](https://xd.adobe.com/view/ee9e607b-dbd6-4372-48dc-b687d32af3da-98af/grid/) +- [Tablet](https://xd.adobe.com/view/d405de00-a871-4f0f-73a0-f8acef30349b-a234/grid/) + +# Understanding the Mock Link: +For understanding the mocks let's take one example. +Open [Admin Auth Mock Link](https://xd.adobe.com/view/0687f00c-695b-437a-79a6-688e7f4f7552-70b6/screen/a841330e-efe5-4bdb-acdd-4d6e52a59571/PC-MP-Admin-Authorization-Add-Profile-Empty-) + +In the below image you can see that there are two icons on left side of the screen. First icon is for comment, using which you can comment on the mock and second icon is for screen details which provide detailed information about the UI. + +Screenshot 2020-01-22 at 3 14 13 PM + +Once you have selected the second icon, you will see a menu on right as below: + +Screenshot 2020-01-22 at 3 17 03 PM + +This menu contains details about the screen width, colors, fonts, etc. Also, please note that the screen width in the mocks is **360 px**. + +# Gathering information about each view +You can click on any item in mock to know more details about that view. For example, in image shown below you can see that as the **Administrator Authorization Required** text is selected, the right side menu displays properties of the textview like font, size, color, transparency, etc. Also, the pink color lines/values in mock displays the margin/padding of that item with respect to other items which helps to set margin/padding in android code. + +Screenshot 2020-01-22 at 3 20 19 PM + +*** + +# Important Points +### Fonts +Mocks uses Roboto fonts which are similar to sans-serif font in the android code. For example, if mock show font-style as `Roboto, Medium`, then android code for that would be `android:fontFamily="sans-serif-medium"` + +### Padding/Margin/View Dimensions +Mocks might contain padding values like 32.5px or 15px but make sure that you use padding/margin and view dimensions in multiple of 4 in android. So 32.5px will get converted to 32dp and 15px will get converted to 16dp. + +### Color Code +Before introducing any new color code in android check if that color code is already present in the `colors.xml` file or not. Also, 8-character hex code on mocks uses **RGBA** convention and 8-character hex code in android is written as **ARGB** where **A** means alpha/transparency and **R**, **G**, **B** stands for red, green, blue color respectively. +For example, if mock contains **#33445566** then in this last two characters are for transparency and first siz are for RGB. So in android it will be written as **#66334455**. + +### Opacity +Do not forget to check the opacity value in mocks. For example you find Opacity value `20%` then in that case you can follow two approaches to use that in code: +1. Use `android:alpha="0.2"` which means 20% opacity in android. +2. Add two more characters at the start of color code of that view, for example if the item color code was `#999999` with 20% opacity value then the color code for that will be `#33999999`. Check [this](https://gist.github.com/lopspower/03fb1cc0ac9f32ef38f4) link for converting percentage to hex value. + +### Blur effect +Currently, we are using blur effect only over the locked chapters of an exploration. So if the mock item does have a blur value then in that case optimise the android code in such a way that it does look almost like the mock without using the blur effect. + +### ImageView Aspect Ratio +ImageView in android code uses aspect ratio of `4:3` or `16:9` mostly, though in some special cases it can be different. So based on the dimensions in mock, calculate the aspect ratio (we can calculate that by getting the width and height from UI and just calculate the ratio which becomes the aspect ratio) and use it accordingly in the code. + +### Bottom Margin in scrollable screens +Almost in all scrollable screens there is bottom margin, make sure that your mock includes that as per mock. + +### TextViews +In all textviews, make sure that proper `android:maxLines` and `android:ellipsize` values are introduced. + +*** + +# Testing UI +For UI testing you can do the following things: +### Manual testing +For this, you can turn on the `Show Layout Bounds` in your mobile **Settings/Developer Mode**. This helps mainly in identifying the boundaries of items and to check if there is any overlapping between the items. + +### Accessibility Scanner +[Accessibility Scanner](https://support.google.com/accessibility/android/answer/6376570?hl=en) can help you identify opportunities to improve your app for users. + +Accessibility Scanner scans your screen and provides suggestions to improve the accessibility of your app, based on the following: + +* Content labels +* Touch target size +* Clickable items +* Text and image contrast + +You can know more about Accessibility in Oppia-Android [here](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide) diff --git a/wiki/Writing-design-docs.md b/wiki/Writing-design-docs.md new file mode 100644 index 00000000000..7207300dc9c --- /dev/null +++ b/wiki/Writing-design-docs.md @@ -0,0 +1,49 @@ +## Instructions +When writing design docs at Oppia, please use this [design doc template](https://docs.google.com/document/d/1mnz8f708DZIa6BpUyRmbb0gCT6EKO3wIvWa_3rOEOYs/edit#). This will ensure that all the necessary information for the project is documented in a central location, and enable project reviews to be done effectively. + +More specifically, here’s how to get started: + +1. Make a copy of [this document](https://docs.google.com/document/d/1mnz8f708DZIa6BpUyRmbb0gCT6EKO3wIvWa_3rOEOYs/edit#). +2. Fill in the sections with details pertaining to your feature/project, following the instructions in the document template. +3. Proofread what you've written before asking reviewers to take a look. + +## How to respond to doc reviews + +When you're responding to a reviewer's comments in a doc, we recommend that you treat it like [responding to a code review](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#tips-for-getting-your-pr-merged-after-submission). + +More specifically: +- Feel free to accept (or reject!) suggestions. If you reject a suggestion, that's fine, but say why. +- In general, treat comments similarly to how you would treat comments in a standard code review. In other words: + - Before asking for a follow-up review, make sure to reply to each comment (maybe with "Done") and update the doc as needed, but **don't resolve the comment thread**. Let the reviewer handle that, so that they can keep track of which comments still need to be addressed. Sometimes, more follow-up might be needed when the reviewer looks at the updated version of the doc. + - If you have any questions about the reviewer's comments, feel free to ask them for clarification. + +## Why we write design documents +Design documents provide team members with the opportunity to review the future implementation of a project before the code has been fully written. A good design document: + +1. Outlines the technical goals of a feature or project that needs to be implemented +2. Describes the architectural changes in the codebase that the project will include +3. Describes how the project will be implemented (specific details of what you plan to build, but not the actual code) +4. Provides a break-down of tasks that will be completed +5. Provides time estimates for each task & how they will fit within defined milestones +6. Considers other aspects of the project, including privacy, security, accessibility, and others + +## Tips when writing design documents + +- Make sure that you understand the high-level goals of the project before going into increasingly lower-level details. +- Use dependency graphs, flow diagrams, and bullet lists when communicating the high-level architectural changes of the project. +- When considering additional options that go beyond the initial goals of the project, consider whether these are essential. If they aren't, add them to a "future work" section that could be worked on alongside or after the project (but not as part of the project itself). If it is essential, make sure you factor that into the implementation plan. +- When breaking down a project, determine all of the tasks that need to be completed for the project. A task is either performing a migration, adding/updating documentation, or creating a PR (each PR should correspond to a single task). +- When estimating how long a task will take for a project, assume it will take longer than you expect. Engineers often have a tendency of assuming implementation will go perfectly, but they sometimes don't and it's difficult to anticipate the things that could go wrong (e.g. bugs are found, a chosen library won't work, etc.). Suggest always multiplying your time estimates by 1.5x or 2x based on past experience (we call this a "fudge factor"). Consider also going back to old projects and comparing how much time you spent versus your estimates--this can help you figure out a good "fudge factor" to use when providing time estimates for future projects. +- If you're unsure how to approach the implementation, start by writing a basic hacked-together prototype to help solve specific questions of the implementation that you're unsure of. Look at other similar features for how they're laid out to compare. You should avoid implementing too much of the project in advance: the main purpose of a design document is to get feedback for a project before spending the large amount of time implementing it. For that reason, the document should take much less time than the project to create. +- If you're unsure about specific parts of the project: call these out as open questions so that other team members can weigh in and provide suggestions or resources that can help you resolve the open questions. +- If there's something you don't fully understand when writing any part of the design document, ask more questions. Sometimes we make mistakes in how we explain things, and that can lead to projects being taken in a direction we didn't anticipate. Other times, we see contributors make assumptions about one particular technical area (such as testing) and describe something other than what we expect. More questions can help bridge any missing knowledge, and can result in changes to our document templates or project goals. +- Use related artifacts when coming up with designs. We have lots of past design docs that can provide more detail on the types of things we value as a team, and how to describe those things. Things that follow established patterns are easier to understand since they minimize the amount of context needed to comprehend it. One way to evaluate how much a particular design document or proposal minimizes context is by considering how much easier/harder it is to understand when compared with other documents describing a project of similar complexity. + +### Additional tips for large projects + +The following tips correspond to projects that span 3+ months: +- Use milestones to organize tasks & specify expected completion times to communicate expectations with the team +- When creating milestones, consider the high-level "deliverables" of a project: what can you demonstrate to someone else after a set of tasks are completed that move Oppia toward the finished result of your project? +- When estimating a milestone, first estimate how much time each task takes and then fit as many tasks as you can within a milestone. Don't change your time estimates based on the milestone (just because something is expected to get done within a certain timeframe doesn't mean it can). If the milestones don't provide enough time to finish the project, that may indicate that the project needs to be rescoped. +- If the project has multiple developers, ensure tasks and milestones have clear owners assigned to them. + diff --git a/wiki/_Footer.md b/wiki/_Footer.md new file mode 100644 index 00000000000..f3e4b194762 --- /dev/null +++ b/wiki/_Footer.md @@ -0,0 +1 @@ +Have an idea for how to improve the wiki? Please help make our documentation better by following our [instructions for contributing to the wiki.](https://github.com/oppia/oppia-android/wiki/Wiki#contributing-to-the-wiki) diff --git a/wiki/_Sidebar.md b/wiki/_Sidebar.md new file mode 100644 index 00000000000..a3f7c429b7b --- /dev/null +++ b/wiki/_Sidebar.md @@ -0,0 +1,55 @@ +**Core documentation** + * [Oppia's mission](https://github.com/oppia/oppia-android/wiki/Our-Mission) + * [Code of Conduct](https://github.com/oppia/oppia-android/blob/develop/.github/CODE_OF_CONDUCT.md) + * **[Get involved!](https://github.com/oppia/oppia-android/wiki/home)** + * [Coders](https://github.com/oppia/oppia-android/wiki/Contributing-to-Oppia-android) + * [Teams at Oppia Android](https://github.com/oppia/oppia-android/wiki/Teams-at-oppia-android) + +--- +**Developing Oppia** + * Contributing to Oppia Android + * [Getting Started](https://github.com/oppia/oppia-android/wiki/Contributing-to-Oppia-Android) + * [Installing Oppia-Android](https://github.com/oppia/oppia-android/wiki/Installing-Oppia-Android) + * [Troubleshooting Installation](https://github.com/oppia/oppia-android/wiki/Troubleshooting-Installation) + * [How to Get Help ]( https://github.com/oppia/oppia-android/wiki/Get-Help) + * [Overview of the Oppia Android codebase and architecture](https://github.com/oppia/oppia-android/wiki/Overview-of-the-Oppia-Android-codebase-and-architecture) + * Bazel + * [Bazel setup instructions](https://github.com/oppia/oppia-android/wiki/Oppia-Bazel-Setup-Instructions) + * [Bazel Setup Instructions for Windows](https://github.com/oppia/oppia-android/wiki/Bazel-Setup-Instructions-for-Windows) + * Key Workflows + * [Guidance on submitting a PR](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR) + * [Creating a screenshot](https://github.com/oppia/oppia-android/wiki/Creating-a-screenshot) + * [Interpreting CI Results](https://github.com/oppia/oppia-android/wiki/Interpreting-CI-Results) + * Testing + * [Oppia Android Testing](https://github.com/oppia/oppia-android/wiki/Oppia-Android-Testing) + * [End to End Testing Guide](https://github.com/oppia/oppia-android/wiki/End-to-End-Testing-Guide) + * [Developing Skills](https://github.com/oppia/oppia-android/wiki/Developing-skills) + * [Frequent Errors and Solutions](https://github.com/oppia/oppia-android/wiki/Frequent-Errors-and-Solutions) + * [RTL Guidelines](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines) + * [Working on UI](https://github.com/oppia/oppia-android/wiki/Working-on-UI) + * [Writing Design Docs](https://github.com/oppia/oppia-android/wiki/Writing-design-docs) +--- +**Developer Reference** + * Code style + * [Coding style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide) + * [Ktlint Guide](https://github.com/oppia/oppia-android/wiki/Ktlint-Guide) + * [Static Analysis Checks](https://github.com/oppia/oppia-android/wiki/Static-Analysis-Checks) + * [Accessibility Guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide) + * [Background Processing](https://github.com/oppia/oppia-android/wiki/Background-Processing) + * [Kotlin Coroutines](https://github.com/oppia/oppia-android/wiki/Kotlin-Coroutines) + * [DataProvider & LiveData](https://github.com/oppia/oppia-android/wiki/DataProvider-&-LiveData) + * [PersistentCacheStore & In Memory Blocking Cache](https://github.com/oppia/oppia-android/wiki/PersistentCacheStore-&-In-Memory-Blocking-Cache) + * [Dark mode](https://github.com/oppia/oppia-android/wiki/Dark-Mode) + * [Buf Guide](https://github.com/oppia/oppia-android/wiki/Buf-Guide) + * [Firebase Console Guide](https://github.com/oppia/oppia-android/wiki/Firebase-Console-Guide) + * [Platform Parameters & Feature Flags](https://github.com/oppia/oppia-android/wiki/Platform-Parameters-&-Feature-Flags) + * [Work Manager](https://github.com/oppia/oppia-android/wiki/Work-Manager) + * [Dependency Injection](https://github.com/oppia/oppia-android/wiki/Dependency-Injection) with [Dagger](https://github.com/oppia/oppia-android/wiki/Dagger) + * [Revert & regression policy](https://github.com/oppia/oppia-android/wiki/Revert-&-regression-policy) + * Bazel + * [Gradle Bazel Migration Best Practices and FAQ](https://github.com/oppia/oppia-android/wiki/Gradle--Bazel-Migration-Best-Practices-and-FAQ) + * [Updating Maven Dependencies](https://github.com/oppia/oppia-android/wiki/Updating-Maven-Dependencies) + * [Internationalization](https://github.com/oppia/oppia-android/wiki/Internationalization) + * Past Events + * Google Summer of Code (note: links are to oppia/oppia): [2021](https://github.com/oppia/oppia/wiki/Google-Summer-of-Code-2021), [2020](https://github.com/oppia/oppia/wiki/Google-Summer-of-Code-2020) + * [SLoP 2020](https://github.com/oppia/oppia-android/wiki/SLoP-2020) \ No newline at end of file