Fix #4451: Add domain level functionality for spotlights #4498

JishnuGoyal · 2022-08-08T18:58:53Z

Explanation

This PR lays the domain-level foundation for "Spotlights". These are a part of the Interactive On-boarding Flow (GSoC) project which focuses on making the initial user experience easier and interactive, and aims to make the user start learning effectively as soon as possible.

spotlight.proto is a model class that holds the information about which spotlights have/have not been seen yet by the user.

SpotlightStateController is an injectable class which controls the domain level functionality needed to make changes to the above model class. Essentially, this class can mark spotlights "seen" and retrieve the spotlight states (where states determine whether they are seen/unseen).

The entire Spotlight experience can be turned on or off just by setting the platform parameter flag called EnableSpotlightUi to true or false.

This PR is backed by necessary tests to check if the domain layer for spotlights works correctly or not.

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).

JishnuGoyal · 2022-08-09T20:47:51Z

Please take an initial pass at this
PTAL @BenHenning

oppiabot · 2022-08-17T22:14:48Z

Hi @JishnuGoyal, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

BenHenning

Thanks @JishnuGoyal! Took a first pass review--PTAL.

domain/src/main/java/org/oppia/android/domain/spotlight/SpotlightStateController.kt

model/src/main/proto/spotlight.proto

oppiabot · 2022-09-03T20:10:15Z

Hi @JishnuGoyal, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

JishnuGoyal · 2022-09-04T19:27:10Z

@BenHenning Please check if this is on the right track; ill be adding tests in my upcoming commit. PTAL

BenHenning

Thanks @JishnuGoyal! I think this is much clearer now! There's one comment you didn't respond to earlier, but otherwise I only had a few more follow-ups. Please go ahead and finish up this PR and send it back when you're ready.

Note also that it appears there are some CI checks failing, as well. I noticed the lint checks are failing in particular--have you run setup.sh locally for your repository? It adds a pre-push hook to check for common lint errors before pushing and helps reviewers avoid needing to look at various nits.

domain/src/main/java/org/oppia/android/domain/spotlight/BUILD.bazel

domain/src/main/java/org/oppia/android/domain/spotlight/SpotlightStateController.kt

model/src/main/proto/spotlight.proto

domain/src/main/java/org/oppia/android/domain/spotlight/BUILD.bazel

JishnuGoyal · 2022-09-09T23:25:47Z

PTAL @BenHenning , thanks!

oppiabot · 2022-09-09T23:28:50Z

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

BenHenning

Thanks @JishnuGoyal. Took another pass--PTAL.

domain/src/main/java/org/oppia/android/domain/spotlight/SpotlightStateController.kt

domain/src/test/java/org/oppia/android/domain/spotlight/SpotlightStateControllerTest.kt

model/src/main/proto/spotlight.proto

domain/src/test/java/org/oppia/android/domain/spotlight/SpotlightStateControllerTest.kt

domain/src/main/java/org/oppia/android/domain/spotlight/SpotlightStateController.kt

JishnuGoyal · 2022-09-10T21:04:00Z

@BenHenning PTAL please take another pass at this, thanks.

oppiabot · 2022-09-10T21:07:03Z

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

BenHenning

Thanks @JishnuGoyal. Other than the CI failures, the PR LGTM. Please assign back once they're fixed.

… into 1.7_domain_layer_for_spotlights # Conflicts: # domain/src/test/java/org/oppia/android/domain/spotlight/SpotlightStateControllerTest.kt

JishnuGoyal · 2022-09-24T06:32:01Z

PTAL @BenHenning

oppiabot · 2022-09-24T06:35:04Z

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

BenHenning

Thanks @JishnuGoyal. Everything LGTM here.

JishnuGoyal added 5 commits July 24, 2022 22:12

add enableSpotlightsUi platform parameter

dcefb48

add spotlight proto

5276321

add spotlight state controller

5ef71bf

small fixes

3e30fbb

lint

6bd3c4f

oppiabot bot assigned BenHenning Aug 9, 2022

Merge branch 'develop' into 1.7_domain_layer_for_spotlights

fa43bb7

oppiabot bot added the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Aug 17, 2022

JishnuGoyal marked this pull request as ready for review August 20, 2022 15:28

JishnuGoyal requested a review from BenHenning as a code owner August 20, 2022 15:28

oppiabot bot removed the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Aug 20, 2022

BenHenning requested changes Aug 24, 2022

View reviewed changes

BenHenning assigned JishnuGoyal and unassigned BenHenning Aug 24, 2022

JishnuGoyal and others added 4 commits August 27, 2022 16:11

suggested changes

84df835

add bazel config for spotlight proto

05e268a

add bazel config for spotlight state controller

2f46252

add tests

ffe0013

oppiabot bot added stale Corresponds to items that haven't seen a recent update and may be automatically closed. and removed stale Corresponds to items that haven't seen a recent update and may be automatically closed. labels Sep 3, 2022

JishnuGoyal added 2 commits September 5, 2022 00:50

add code documentation

2246ef9

remove extra eof and add some documentation

68c4b0b

JishnuGoyal assigned BenHenning and unassigned JishnuGoyal Sep 4, 2022

BenHenning requested changes Sep 5, 2022

View reviewed changes

BenHenning assigned JishnuGoyal Sep 5, 2022

oppiabot bot assigned BenHenning and unassigned JishnuGoyal Sep 9, 2022

BenHenning reviewed Sep 10, 2022

View reviewed changes

BenHenning assigned JishnuGoyal and unassigned BenHenning Sep 10, 2022

JishnuGoyal added 4 commits September 10, 2022 23:10

commit requested changes

be6de62

lint

a8a1aa8

Merge branch 'develop' into 1.7_domain_layer_for_spotlights

092da3b

changes acc to updates to the persistent cache store

07b408c

oppiabot bot assigned BenHenning and unassigned JishnuGoyal Sep 10, 2022

BenHenning reviewed Sep 12, 2022

View reviewed changes

BenHenning assigned JishnuGoyal and unassigned BenHenning Sep 12, 2022

JishnuGoyal and others added 6 commits September 18, 2022 03:15

bazel config

2fd6881

lint

5a9ebd5

use spotlight_java_proto_lite

072f0d5

lint

85abe09

Merge remote-tracking branch 'origin/1.7_domain_layer_for_spotlights'…

7ee3867

… into 1.7_domain_layer_for_spotlights # Conflicts: # domain/src/test/java/org/oppia/android/domain/spotlight/SpotlightStateControllerTest.kt

bazel lint

3010218

oppiabot bot assigned BenHenning and unassigned JishnuGoyal Sep 24, 2022

BenHenning approved these changes Sep 24, 2022

View reviewed changes

BenHenning merged commit 7653898 into oppia:develop Sep 24, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #4451: Add domain level functionality for spotlights #4498

Fix #4451: Add domain level functionality for spotlights #4498

JishnuGoyal commented Aug 8, 2022 •

edited

JishnuGoyal commented Aug 9, 2022

oppiabot bot commented Aug 17, 2022

BenHenning left a comment

oppiabot bot commented Sep 3, 2022

JishnuGoyal commented Sep 4, 2022

BenHenning left a comment

JishnuGoyal commented Sep 9, 2022

oppiabot bot commented Sep 9, 2022

BenHenning left a comment

JishnuGoyal commented Sep 10, 2022

oppiabot bot commented Sep 10, 2022

BenHenning left a comment

JishnuGoyal commented Sep 24, 2022

oppiabot bot commented Sep 24, 2022

BenHenning left a comment

Fix #4451: Add domain level functionality for spotlights #4498

Fix #4451: Add domain level functionality for spotlights #4498

Conversation

JishnuGoyal commented Aug 8, 2022 • edited

Explanation

Essential Checklist

JishnuGoyal commented Aug 9, 2022

oppiabot bot commented Aug 17, 2022

BenHenning left a comment

Choose a reason for hiding this comment

oppiabot bot commented Sep 3, 2022

JishnuGoyal commented Sep 4, 2022

BenHenning left a comment

Choose a reason for hiding this comment

JishnuGoyal commented Sep 9, 2022

oppiabot bot commented Sep 9, 2022

BenHenning left a comment

Choose a reason for hiding this comment

JishnuGoyal commented Sep 10, 2022

oppiabot bot commented Sep 10, 2022

BenHenning left a comment

Choose a reason for hiding this comment

JishnuGoyal commented Sep 24, 2022

oppiabot bot commented Sep 24, 2022

BenHenning left a comment

Choose a reason for hiding this comment

JishnuGoyal commented Aug 8, 2022 •

edited