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.

Sign up for GitHub

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

Jump to bottom

GaeSolution's 'correct_answer' field should be 'Any?' instead of 'String?' #1547

Open
miaboloix opened this issue Jul 30, 2020 · 5 comments · May be fixed by #4885
Open

GaeSolution's 'correct_answer' field should be 'Any?' instead of 'String?' #1547

miaboloix opened this issue Jul 30, 2020 · 5 comments · May be fixed by #4885
Assignees
@BenHenning
Labels
bug End user-perceivable behaviors which are not desirable. Impact: Low Low perceived user impact (e.g. edge cases). Issue: Needs Clarification Indicates that an issue needs more detail in order to be able to be acted upon. Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet. Z-ibt Temporary label for Ben to keep track of issues he's triaged.
Projects
Bazel Migration

Comments

@miaboloix
Copy link
Contributor

Is your feature request related to a problem? Please describe.
According to state_domain.py, correct_answer in GaeSolution.kt should not be typecast as 'String' but as 'Any'.

Describe the solution you'd like
The type declared in GaeSolution.kt for correct_answer must be changed to Any?. On top of this, in exploration.proto we must cast correct_answer to be an InteractionObject and must change StateRetreiver.kt in order to parse the field correctly.
@miaboloix miaboloix self-assigned this Jul 30, 2020
@miaboloix miaboloix mentioned this issue Jul 30, 2020
Closed
8 tasks
@miaboloix miaboloix added this to To do in Bazel Migration Aug 10, 2020
@miaboloix miaboloix moved this from To do: Stage 2 to To do: in Bazel Migration Aug 10, 2020
@miaboloix miaboloix added this to the Stage 2 of Bazel Migration milestone Aug 10, 2020
@BenHenning BenHenning assigned BenHenning and unassigned miaboloix Feb 19, 2021
@seanlip seanlip moved this from To do (Stage 1): to To do (Stage 2) in Bazel Migration Aug 12, 2021
@Broppia Broppia added issue_type_bug Impact: Low Low perceived user impact (e.g. edge cases). labels Jul 29, 2022
@BenHenning BenHenning added issue_type_infrastructure Issue: Needs Clarification Indicates that an issue needs more detail in order to be able to be acted upon. Z-ibt Temporary label for Ben to keep track of issues he's triaged. and removed issue_type_bug labels Sep 15, 2022
@BenHenning BenHenning removed this from the Stage 2 of Bazel Migration milestone Sep 16, 2022
@seanlip seanlip added bug End user-perceivable behaviors which are not desirable. and removed issue_type_infrastructure labels Mar 28, 2023
@seanlip
Copy link
Member

seanlip commented Mar 28, 2023 •
edited

Actually, it should be cast to a union of possible answer types -- Any is a bit broad. Note that the typing in Oppia-Web (see link in the issue description) has been made stricter since this issue was filed.

@BenHenning
Copy link
Sponsor Member

This will actually become obsolete in #4885 because that will be removing the JSON endpoints from the app proper and basically shift the app entirely over to using protos only (which also are a bit non-specific in terms of solution typing, but that's fully fixed in oppia/oppia-proto-api#1).

@seanlip
Copy link
Member

seanlip commented Apr 4, 2023

@BenHenning #4885 is closed, is that intended?

Also, based on your comment, can we just close this issue, or add "Fix #1547" to the PR title of #4885 if we still want to keep it open?

@BenHenning BenHenning linked a pull request Apr 6, 2023 that will close this issue
Draft
6 tasks
@BenHenning
Copy link
Sponsor Member

BenHenning commented Apr 6, 2023 •
edited

@seanlip #4885 will be reopened once I have time to focus on it again, but I won't lose track of it (I'd rather not keep it open only to have Oppiabot close it again, though; currently load-balancing 7 PRs & counting).

I added a reference in the PR to this issue, and I will for sure update it once I finalize the PR to make it more clear (as I've done with past issues).

@seanlip
Copy link
Member

seanlip commented Apr 6, 2023

OK, just having the reference is good enough so that this gets auto-closed at the right time. Thanks!

@MohitGupta121 MohitGupta121 added the Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet. label Jun 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@BenHenning BenHenning

Labels
bug End user-perceivable behaviors which are not desirable. Impact: Low Low perceived user impact (e.g. edge cases). Issue: Needs Clarification Indicates that an issue needs more detail in order to be able to be acted upon. Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet. Z-ibt Temporary label for Ben to keep track of issues he's triaged.
Projects
[Task Force] Migrate to Bazel - Android
Status: Todo
[Team] Developer Workflow & Infrastructure - Android
Status: In Progress
Bazel Migration
  
To do (Stage 2)
Milestone
No milestone
5 participants
@seanlip @BenHenning @miaboloix @MohitGupta121 @Broppia