Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Develop (fork) Branch Update #5

Merged
merged 5 commits into from
Sep 16, 2024
Merged

Commits on Sep 5, 2024

  1. Fix #1730 : Prevent binary files from being checked in using pre-comm…

    …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
    
    ![image](https://github.com/user-attachments/assets/b953b2d7-55ec-46e6-a0b9-00967200509c)
    
    ### 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)
    ]
    
    ![image](https://github.com/user-attachments/assets/64352473-86cb-49e0-b888-d3247286e9e5)
    
    ## 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)).
    Rd4dev authored Sep 5, 2024
    Configuration menu
    Copy the full SHA
    4730edb View commit details
    Browse the repository at this point in the history

Commits on Sep 10, 2024

  1. Fix #5357: Remove CDATA from translatable strings (#5524)

    <!-- 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
    adhiamboperes authored Sep 10, 2024
    Configuration menu
    Copy the full SHA
    4f4831a View commit details
    Browse the repository at this point in the history

Commits on Sep 12, 2024

  1. Fix part of #5508: Limit APK/AAB Difference analysis reports in the P…

    …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]>
    Rd4dev and BenHenning authored Sep 12, 2024
    Configuration menu
    Copy the full SHA
    c82b71b View commit details
    Browse the repository at this point in the history

Commits on Sep 16, 2024

  1. Fix #5485 Create means for verifying Fragment Arguments (#5527)

    <!-- 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]>
    subhajitxyz and theMr17 authored Sep 16, 2024
    Configuration menu
    Copy the full SHA
    65197bf View commit details
    Browse the repository at this point in the history
  2. Fix #5344: Remove temporary functions from TopicListController (#5528)

    <!-- 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
    theMr17 authored Sep 16, 2024
    Configuration menu
    Copy the full SHA
    aadc1a1 View commit details
    Browse the repository at this point in the history