Fix #3890 Break *Required into 2 parts. #4019

rishidyno · 2021-11-30T20:10:55Z

Explanation

LTR

Before	After

RTL

Before	After

Essential Checklist

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 files have their rationale included in the PR explanation.
The PR follows the style guide.
The PR does not contain any unnecessary code changes from Android Studio (reference).
The PR is made from a branch that's not called "develop" and is up-to-date with "develop".
The PR is assigned to the appropriate reviewers (reference).

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)
Add a video showing the full UX flow with a screen reader enabled (see accessibility guide)
Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing

rishidyno · 2021-12-01T08:08:03Z

@BenHenning I am getting this error. I tried adding app_language_resource_handler dependency in the app BUILD.bazel file.

This is the error log

Starting local Bazel server and connecting to it...
INFO: Analyzed target //:oppia (280 packages loaded, 5615 targets configured).
INFO: Found 1 target...
ERROR: /home/rishi/opensource/oppia-android/app/BUILD.bazel:628:19: KotlinCompile //app:view_models_kt { kt: 156, java: 0, srcjars: 0 } for armeabi-v7a failed: (Exit 1): builder failed: error executing command bazel-out/host/bin/external/io_bazel_rules_kotlin/src/main/kotlin/builder '--flagfile=bazel-out/android-armeabi-v7a-fastbuild/bin/app/view_models_kt-kt-java.jar-0.params'
warning: runtime JAR files in the classpath should have the same version. These files were found in the classpath:
/home/rishi/.cache/bazel/_bazel_rishi/2d9bbf034eeaafac68b4112f599a43d0/execroot/main/external/com_github_jetbrains_kotlin/lib/kotlin-stdlib.jar (version 1.4)
/home/rishi/.cache/bazel/_bazel_rishi/2d9bbf034eeaafac68b4112f599a43d0/execroot/main/external/com_github_jetbrains_kotlin/lib/kotlin-stdlib-jdk7.jar (version 1.4)
/home/rishi/.cache/bazel/_bazel_rishi/2d9bbf034eeaafac68b4112f599a43d0/execroot/main/external/com_github_jetbrains_kotlin/lib/kotlin-stdlib-jdk8.jar (version 1.4)
/home/rishi/.cache/bazel/_bazel_rishi/2d9bbf034eeaafac68b4112f599a43d0/execroot/main/bazel-out/android-armeabi-v7a-fastbuild/bin/external/maven/v1/https/repo1.maven.org/maven2/org/jetbrains/kotlin/kotlin-stdlib/1.5.0/header_kotlin-stdlib-1.5.0.jar (version 1.5)
/home/rishi/.cache/bazel/_bazel_rishi/2d9bbf034eeaafac68b4112f599a43d0/execroot/main/bazel-out/android-armeabi-v7a-fastbuild/bin/external/maven/v1/https/repo1.maven.org/maven2/org/jetbrains/kotlin/kotlin-stdlib-common/1.5.0/header_kotlin-stdlib-common-1.5.0.jar (version 1.5)
/home/rishi/.cache/bazel/_bazel_rishi/2d9bbf034eeaafac68b4112f599a43d0/execroot/main/bazel-out/android-armeabi-v7a-fastbuild/bin/external/maven/v1/https/repo1.maven.org/maven2/org/jetbrains/kotlin/kotlin-reflect/1.5.0/header_kotlin-reflect-1.5.0.jar (version 1.5)
/home/rishi/.cache/bazel/_bazel_rishi/2d9bbf034eeaafac68b4112f599a43d0/execroot/main/bazel-out/android-armeabi-v7a-fastbuild/bin/external/maven/v1/https/repo1.maven.org/maven2/org/jetbrains/kotlin/kotlin-stdlib-jdk8/1.4.10/header_kotlin-stdlib-jdk8-1.4.10.jar (version 1.4)
/home/rishi/.cache/bazel/_bazel_rishi/2d9bbf034eeaafac68b4112f599a43d0/execroot/main/bazel-out/android-armeabi-v7a-fastbuild/bin/external/maven/v1/https/repo1.maven.org/maven2/org/jetbrains/kotlin/kotlin-stdlib-jdk7/1.4.10/header_kotlin-stdlib-jdk7-1.4.10.jar (version 1.4)
/home/rishi/.cache/bazel/_bazel_rishi/2d9bbf034eeaafac68b4112f599a43d0/execroot/main/external/com_github_jetbrains_kotlin/lib/kotlin-script-runtime.jar (version 1.4)
/home/rishi/.cache/bazel/_bazel_rishi/2d9bbf034eeaafac68b4112f599a43d0/execroot/main/external/com_github_jetbrains_kotlin/lib/kotlin-reflect.jar (version 1.4)
warning: some runtime JAR files in the classpath have an incompatible version. Consider removing them from the classpath
warning: runtime JAR files in the classpath should have the same version. These files were found in the classpath:
/home/rishi/.cache/bazel/_bazel_rishi/2d9bbf034eeaafac68b4112f599a43d0/execroot/main/external/com_github_jetbrains_kotlin/lib/kotlin-stdlib.jar (version 1.4)
/home/rishi/.cache/bazel/_bazel_rishi/2d9bbf034eeaafac68b4112f599a43d0/execroot/main/external/com_github_jetbrains_kotlin/lib/kotlin-stdlib-jdk7.jar (version 1.4)
/home/rishi/.cache/bazel/_bazel_rishi/2d9bbf034eeaafac68b4112f599a43d0/execroot/main/external/com_github_jetbrains_kotlin/lib/kotlin-stdlib-jdk8.jar (version 1.4)
/home/rishi/.cache/bazel/_bazel_rishi/2d9bbf034eeaafac68b4112f599a43d0/execroot/main/bazel-out/android-armeabi-v7a-fastbuild/bin/external/maven/v1/https/repo1.maven.org/maven2/org/jetbrains/kotlin/kotlin-stdlib/1.5.0/header_kotlin-stdlib-1.5.0.jar (version 1.5)
/home/rishi/.cache/bazel/_bazel_rishi/2d9bbf034eeaafac68b4112f599a43d0/execroot/main/bazel-out/android-armeabi-v7a-fastbuild/bin/external/maven/v1/https/repo1.maven.org/maven2/org/jetbrains/kotlin/kotlin-stdlib-common/1.5.0/header_kotlin-stdlib-common-1.5.0.jar (version 1.5)
/home/rishi/.cache/bazel/_bazel_rishi/2d9bbf034eeaafac68b4112f599a43d0/execroot/main/bazel-out/android-armeabi-v7a-fastbuild/bin/external/maven/v1/https/repo1.maven.org/maven2/org/jetbrains/kotlin/kotlin-reflect/1.5.0/header_kotlin-reflect-1.5.0.jar (version 1.5)
/home/rishi/.cache/bazel/_bazel_rishi/2d9bbf034eeaafac68b4112f599a43d0/execroot/main/bazel-out/android-armeabi-v7a-fastbuild/bin/external/maven/v1/https/repo1.maven.org/maven2/org/jetbrains/kotlin/kotlin-stdlib-jdk8/1.4.10/header_kotlin-stdlib-jdk8-1.4.10.jar (version 1.4)
/home/rishi/.cache/bazel/_bazel_rishi/2d9bbf034eeaafac68b4112f599a43d0/execroot/main/bazel-out/android-armeabi-v7a-fastbuild/bin/external/maven/v1/https/repo1.maven.org/maven2/org/jetbrains/kotlin/kotlin-stdlib-jdk7/1.4.10/header_kotlin-stdlib-jdk7-1.4.10.jar (version 1.4)
/home/rishi/.cache/bazel/_bazel_rishi/2d9bbf034eeaafac68b4112f599a43d0/execroot/main/external/com_github_jetbrains_kotlin/lib/kotlin-script-runtime.jar (version 1.4)
/home/rishi/.cache/bazel/_bazel_rishi/2d9bbf034eeaafac68b4112f599a43d0/execroot/main/external/com_github_jetbrains_kotlin/lib/kotlin-reflect.jar (version 1.4)
warning: some runtime JAR files in the classpath have an incompatible version. Consider removing them from the classpath
app/src/main/java/org/oppia/android/app/profile/AddProfileViewModel.kt:25:79: error: unresolved reference: add_profile_required
val starRequired: String = "" + resourceHandler.getStringInLocale(R.string.add_profile_required)
^
Compilation failure: compile phase failed:
warning: runtime JAR files in the classpath should have the same version. These files were found in the classpath:
/home/rishi/.cache/bazel/_bazel_rishi/2d9bbf034eeaafac68b4112f599a43d0/execroot/main/external/com_github_jetbrains_kotlin/lib/kotlin-stdlib.jar (version 1.4)
/home/rishi/.cache/bazel/_bazel_rishi/2d9bbf034eeaafac68b4112f599a43d0/execroot/main/external/com_github_jetbrains_kotlin/lib/kotlin-stdlib-jdk7.jar (version 1.4)
/home/rishi/.cache/bazel/_bazel_rishi/2d9bbf034eeaafac68b4112f599a43d0/execroot/main/external/com_github_jetbrains_kotlin/lib/kotlin-stdlib-jdk8.jar (version 1.4)
/home/rishi/.cache/bazel/_bazel_rishi/2d9bbf034eeaafac68b4112f599a43d0/execroot/main/bazel-out/android-armeabi-v7a-fastbuild/bin/external/maven/v1/https/repo1.maven.org/maven2/org/jetbrains/kotlin/kotlin-stdlib/1.5.0/header_kotlin-stdlib-1.5.0.jar (version 1.5)
/home/rishi/.cache/bazel/_bazel_rishi/2d9bbf034eeaafac68b4112f599a43d0/execroot/main/bazel-out/android-armeabi-v7a-fastbuild/bin/external/maven/v1/https/repo1.maven.org/maven2/org/jetbrains/kotlin/kotlin-stdlib-common/1.5.0/header_kotlin-stdlib-common-1.5.0.jar (version 1.5)
/home/rishi/.cache/bazel/_bazel_rishi/2d9bbf034eeaafac68b4112f599a43d0/execroot/main/bazel-out/android-armeabi-v7a-fastbuild/bin/external/maven/v1/https/repo1.maven.org/maven2/org/jetbrains/kotlin/kotlin-reflect/1.5.0/header_kotlin-reflect-1.5.0.jar (version 1.5)
/home/rishi/.cache/bazel/_bazel_rishi/2d9bbf034eeaafac68b4112f599a43d0/execroot/main/bazel-out/android-armeabi-v7a-fastbuild/bin/external/maven/v1/https/repo1.maven.org/maven2/org/jetbrains/kotlin/kotlin-stdlib-jdk8/1.4.10/header_kotlin-stdlib-jdk8-1.4.10.jar (version 1.4)
/home/rishi/.cache/bazel/_bazel_rishi/2d9bbf034eeaafac68b4112f599a43d0/execroot/main/bazel-out/android-armeabi-v7a-fastbuild/bin/external/maven/v1/https/repo1.maven.org/maven2/org/jetbrains/kotlin/kotlin-stdlib-jdk7/1.4.10/header_kotlin-stdlib-jdk7-1.4.10.jar (version 1.4)
/home/rishi/.cache/bazel/_bazel_rishi/2d9bbf034eeaafac68b4112f599a43d0/execroot/main/external/com_github_jetbrains_kotlin/lib/kotlin-script-runtime.jar (version 1.4)
/home/rishi/.cache/bazel/_bazel_rishi/2d9bbf034eeaafac68b4112f599a43d0/execroot/main/external/com_github_jetbrains_kotlin/lib/kotlin-reflect.jar (version 1.4)
warning: some runtime JAR files in the classpath have an incompatible version. Consider removing them from the classpath
/home/rishi/.cache/bazel/_bazel_rishi/2d9bbf034eeaafac68b4112f599a43d0/execroot/main/app/src/main/java/org/oppia/android/app/profile/AddProfileViewModel.kt:25:79: error: unresolved reference: add_profile_required
val starRequired: String = "" + resourceHandler.getStringInLocale(R.string.add_profile_required)
^
Target //:oppia failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 33.654s, Critical Path: 23.36s
INFO: 3 processes: 2 internal, 1 linux-sandbox.
FAILED: Build did NOT complete successfully
rishi@r:~/opensource/oppia-android$

BenHenning · 2021-12-07T20:38:05Z

@rishidyno it looks like you addressed that problem by moving the view model to the VIEW_MODELS_WITH_RESOURCES list. However, you're also using a kotlinx import which shouldn't be used per https://developer.android.com/topic/libraries/view-binding/migration. You should use a direct resource reference, instead, and retrieve the view using findViewById.

…k-required-string

rishidyno · 2021-12-10T19:02:34Z

@BenHenning @rt4914 All tests have passed PTAL.

oppiabot · 2021-12-10T19:05:37Z

Unassigning @rishidyno since a re-review was requested. @rishidyno, please make sure you have addressed all review comments. Thanks!

BenHenning

Thanks @rishidyno! Took a full pass on the PR. Besides the comments I left in code, could you also please update your PR description to:

Include an explanation for what this PR is doing, and some context on the approach you're taking
To include a filled-in "For UI-specific PRs only" section (you should follow all the instructions list)
Under the 'For UI-specific PRs only' please also include at least one screenshot with Arabic to demonstrate the RTL behavior for this string (I want to make sure it reverses correctly)

app/src/main/java/org/oppia/android/app/profile/AddProfileViewModel.kt

app/src/sharedTest/java/org/oppia/android/app/profile/AddProfileActivityTest.kt

rishidyno · 2021-12-14T12:30:20Z

@BenHenning I have added the SS and have addressed the changes PTAL.

BenHenning

Thanks @rishidyno. This LGTM--nothing else to add!

BenHenning · 2021-12-16T01:12:04Z

I'm force merging this since @rt4914 is out & I'm more or less covering his codeowners this week. I reviewed the whole PR & everything LGTM, plus tests are passing.

rishidyno · 2021-12-16T03:55:47Z

Thanks @BenHenning

initial commit.

7d2424c

rishidyno requested review from BenHenning and rt4914 as code owners November 30, 2021 20:10

rishidyno changed the title ~~Break *Required into 2 parts.~~ Fix #3890 Break *Required into 2 parts. Nov 30, 2021

made few changes

e58a21d

BenHenning assigned rishidyno Dec 7, 2021

rishidyno and others added 8 commits December 8, 2021 18:06

removed kotlinx import from AddProfileActivityTest.kt

8791b9f

code formatting

46274d6

code formatting

a871ca6

updated app BUILD.bazel

a1aa0a6

Merge remote-tracking branch 'origin/break-required-string' into brea…

598d120

…k-required-string

code formatting

28e9082

Merge branch 'develop' into break-required-string

fb4bbe8

Update AddProfileViewModel.kt

84c49b2

oppiabot bot assigned BenHenning and unassigned rishidyno Dec 10, 2021

BenHenning requested changes Dec 14, 2021

View reviewed changes

BenHenning assigned rishidyno and unassigned BenHenning Dec 14, 2021

rishidyno and others added 3 commits December 14, 2021 13:27

Merge branch 'oppia:develop' into break-required-string

6aa156e

nitfixes

41b8614

nitfix

dc2b965

rishidyno assigned BenHenning Dec 14, 2021

BenHenning approved these changes Dec 16, 2021

View reviewed changes

BenHenning merged commit a285c0b into oppia:develop Dec 16, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #3890 Break *Required into 2 parts. #4019

Fix #3890 Break *Required into 2 parts. #4019

rishidyno commented Nov 30, 2021 •

edited

rishidyno commented Dec 1, 2021 •

edited

BenHenning commented Dec 7, 2021

rishidyno commented Dec 10, 2021 •

edited

oppiabot bot commented Dec 10, 2021

BenHenning left a comment

rishidyno commented Dec 14, 2021 •

edited

BenHenning left a comment

BenHenning commented Dec 16, 2021

rishidyno commented Dec 16, 2021

Fix #3890 Break *Required into 2 parts. #4019

Fix #3890 Break *Required into 2 parts. #4019

Conversation

rishidyno commented Nov 30, 2021 • edited

Explanation

Essential Checklist

For UI-specific PRs only

rishidyno commented Dec 1, 2021 • edited

BenHenning commented Dec 7, 2021

rishidyno commented Dec 10, 2021 • edited

oppiabot bot commented Dec 10, 2021

BenHenning left a comment

Choose a reason for hiding this comment

rishidyno commented Dec 14, 2021 • edited

BenHenning left a comment

Choose a reason for hiding this comment

BenHenning commented Dec 16, 2021

rishidyno commented Dec 16, 2021

rishidyno commented Nov 30, 2021 •

edited

rishidyno commented Dec 1, 2021 •

edited

rishidyno commented Dec 10, 2021 •

edited

rishidyno commented Dec 14, 2021 •

edited