Develop (fork) Branch Update #5
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…it hook (#5525) <!-- READ ME FIRST: Please fill in the explanation section below and check off every point from the Essential Checklist! --> ## Explanation <!-- - Explain what your PR does. If this PR fixes an existing bug, please include - "Fixes #bugnum:" in the explanation so that GitHub can auto-close the issue - when this PR is merged. --> Fixes #1730 ### The PR includes - A pre-commit hook that identifies binary files in both - staged changes (for local checks before committing) - committed files (for CI checks after pushing to a PR) - If binary files are found, the commit/check is blocked, and the offending files are listed for removal. - The pre-commit hook is integrated via a setup script. - The same script is utilized for the CI pipeline. - The 'Pass' statement is only included in the CI checks to keep the local commit process cleaner. ### Local block as pre-commit hook when a binary file is detected  ### CI re-check if the PR includes a binary - [ Fail - [stack trace](https://github.com/Rd4dev/oppia-android/actions/runs/10715972847/job/29712474511#step:7:15) ] [ Pass - [stack trace](https://github.com/Rd4dev/oppia-android/actions/runs/10716040065/job/29712695824?pr=11#step:7:10) ]  ## Essential Checklist <!-- Please tick the relevant boxes by putting an "x" in them. --> - [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)).
<!-- READ ME FIRST: Please fill in the explanation section below and check off every point from the Essential Checklist! --> ## Explanation Fixes #5357 This PR includes the changes previously included in #5361, with an additional fix for missing closing html tags described in #5403: ``` faq_answer_app_language: Missing </strong> tag after the email address. faq_answer_bug_reporting: Missing closing </ol> tag. faq_answer_exploration_player: Missing closing </ul> tag. faq_answer_update_app: Missing closing </ol> tag and closing </p> tag. faq_answer_update_os: Missing closing </ol> tag and closing </p> tag. ``` This PR removes all CDATA declarations in the translatable strings.xml file and instead escapes all necessary characters: < and > (& didn't need to be escaped since no strings use that character at the moment). This is needed because Translatewiki doesn't seem to extract the HTML within the CDATA declaration correctly, so it may not be translated (some existing strings were never translated, and per #5274 the latest FAQ changes aren't being processed correctly (leading to empty translation strings being submitted). This PR also introduces a regex check + test to ensure that CDATA isn't used anymore in strings. ## Essential Checklist <!-- Please tick the relevant boxes by putting an "x" in them. --> - [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 <!-- Delete these section if this PR does not include UI-related changes. --> 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)) - For PRs introducing new UI elements or color changes, both light and dark mode screenshots must be included - Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing
…R Comment Thread (#5532) <!-- READ ME FIRST: Please fill in the explanation section below and check off every point from the Essential Checklist! --> ## Explanation <!-- - Explain what your PR does. If this PR fixes an existing bug, please include - "Fixes #bugnum:" in the explanation so that GitHub can auto-close the issue - when this PR is merged. --> Fixes part of #5508 ### This PR includes Steps to locate the previous `stats.yml` workflow run, download its build artifact, and compare it with the current build log. If changes are detected, a comment will be uploaded to help minimize comment thread overload. **The implementation:** - Download the previous build log artifact (if available). - Run the script. - Compare the current generated build log with the previous build log artifact: - if no differences are found -> skip commenting. - if differences are found -> comment the current generated build log - if no previous artifact is found -> comment the current generated build log - This occurs in 2 instances: - 1. It's the first run of the PR. - 2. An error occurred during the previous stat check (since the previous build is from the second-to-last run ID). - Upload the current build log as an artifact (for the next stat run). - Comment/skip the stat report based on the comparison result. # ### Tested with a cloned PR _(with stats.yml implementation on develop)_ Tested PR: Rd4dev/Oppia-Android-Fork-from-Fork#40 Reference for proof of implementation: - [x] should comment on initial run | [comment](Rd4dev/Oppia-Android-Fork-from-Fork#40 (comment)) | [stack trace](https://github.com/Rd4dev/Oppia-Android-Fork-from-Fork/actions/runs/10762876260/job/29843752198#step:19:26) - [x] shouldn't comment when no change | [reference1](Rd4dev/Oppia-Android-Fork-from-Fork#40 (comment)) | [reference2](Rd4dev/Oppia-Android-Fork-from-Fork#40 (comment)) - [x] should comment on change | [comment](Rd4dev/Oppia-Android-Fork-from-Fork#40 (comment)) | [reference](Rd4dev/Oppia-Android-Fork-from-Fork#40 (comment)) - [x] comment on previous build fail (replicated!) | [comment](Rd4dev/Oppia-Android-Fork-from-Fork#40 (comment)) | [reference](Rd4dev/Oppia-Android-Fork-from-Fork#40 (comment)) ## Essential Checklist <!-- Please tick the relevant boxes by putting an "x" in them. --> - [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)). --------- Co-authored-by: Ben Henning <[email protected]>
<!-- READ ME FIRST: Please fill in the explanation section below and check off every point from the Essential Checklist! --> ## Explanation Fixes #5485 Added test for 10 fragments arguments and saveInstanceState. ## Essential Checklist <!-- Please tick the relevant boxes by putting an "x" in them. --> - [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: ...".) - [ ] 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 <!-- Delete these section if this PR does not include UI-related changes. --> 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)) - For PRs introducing new UI elements or color changes, both light and dark mode screenshots must be included - Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing --------- Co-authored-by: Mr. 17 <[email protected]>
<!-- READ ME FIRST: Please fill in the explanation section below and check off every point from the Essential Checklist! --> ## Explanation <!-- - Explain what your PR does. If this PR fixes an existing bug, please include - "Fixes #bugnum:" in the explanation so that GitHub can auto-close the issue - when this PR is merged. --> Fixes #5344 This PR surfaces the `getClassrooms` and `getClassroomById` functions from the `ClassroomController`, adds tests to ensure their correctness, and refactors the `TopicListController` to replace temporary functions with these new implementations. ## Essential Checklist <!-- Please tick the relevant boxes by putting an "x" in them. --> - [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 <!-- Delete these section if this PR does not include UI-related changes. --> 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)) - For PRs introducing new UI elements or color changes, both light and dark mode screenshots must be included - Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing
Coverage ReportResultsNumber of files assessed: 12 Passing coverageFiles with passing code coverage
Exempted coverageFiles exempted from coverage
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Explanation
Essential Checklist
For UI-specific PRs only
If your PR includes UI-related changes, then: