Fix part of #13162: StateHitEventHandler #14326

anurag629 · 2021-11-27T19:56:16Z

Overview

This PR fixes or fixes part of Write schemas for handler class arguments #13162.
This PR does the following: This PR adds schema for 'StateHitEventHandler'

Essential Checklist

The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
The linter/Karma presubmit checks have passed locally on your machine.
"Allow edits from maintainers" is checked. (See here for instructions on how to enable it.)
- This lets reviewers restart your CircleCI tests for you.
The PR is made from a branch that's not called "develop".

Proof that changes are correct

PR Pointers

Make sure to follow the instructions for making a code change.
Oppiabot will notify you when you don't add a PR_CHANGELOG label. If you are unable to do so, please @-mention a code owner (who will be in the Reviewers list), or ask on Gitter.
For what code owners will expect, see the Code Owner's wiki page.
Make sure your PR follows conventions in the style guide, otherwise this will lead to review delays.
Never force push. If you do, your PR will be closed.
Oppiabot can assign anyone for review/help if you leave a comment like the following: "{{Question/comment}} @{{reviewer_username}} PTAL"
Some of the e2e tests are flaky, and can fail for reasons unrelated to your PR. We are working on fixing this, but in the meantime, if you need to restart the tests, please check the "If your build fails" wiki page.

…ppia#14263) * Revert "Revert "Fix part of oppia#9749: Create a component that shows rte-output-display and use it instead of angular-html-bind. (oppia#13229)" (oppia#13361)" This reverts commit 792de45. * Fix issues * Fix spelling mistakes in comment * Merge with upstream/develop * Address review comments: 1. Move oppia-rte-output.spec from the list of ignored files for innerHTML check in eslintrc and move to inline disables. 2. Change the CODEOWNER listing from oppia-rte-parser.service.ts to oppia-rte-parser.service*.ts to include the spec file. 3. Add refernce to the note at the top of file for all value.replace ops. 4. Miscellaneous changes. Co-authored-by: Vasa Srijan Reddy <redvasa@amazon.com>

oppiabot · 2021-11-27T19:56:18Z

Hi @anurag629, can you complete the following:

The karma and linter checklist has not been checked, please make sure to run the frontend tests and lint tests before pushing.
The proof that changes are correct has not been provided, please make sure to upload a image/video showing that the changes are correct. Or include a sentence saying "No proof of changes needed because" and the reason why proof of changes cannot be provided.
Thanks!

oppiabot · 2021-11-27T19:56:30Z

Hi @aks681, could you please add the appropriate changelog label to this pull request? Thanks!

anurag629 · 2021-11-27T19:57:23Z

@vojtechjelinek Please look through changes and suggest what to do next

vojtechjelinek · 2021-11-28T18:19:24Z

@anurag629 The backend tests are failing, also you didn't remove the StateHitEventHandler from the schema constants.

vojtechjelinek · 2021-11-29T10:47:26Z

@anurag629 The tests are still failing.

anurag629 · 2021-11-29T11:32:01Z

@anurag629 The tests are still failing.

@vojtechjelinek please help me in this

vojtechjelinek

Left a few comments.

vojtechjelinek · 2021-11-29T12:42:16Z

core/controllers/reader.py

+        'username': {
+            'schema': {
+                'type': 'basestring',
+                'validators': [{
+                    'id': 'is_valid_username_string'
+                }]
+            }
+        }


Why username? When the arg of the post function is exploration_id?

vojtechjelinek · 2021-11-29T12:42:57Z

core/controllers/reader.py

+        }
+    }
+    HANDLER_ARGS_SCHEMAS = {
+        'POST': {}


In the post there are other args that should be defined here.

oppiabot · 2021-11-29T12:46:05Z

Hi @anurag629, it looks like some changes were requested on this pull request by @vojtechjelinek. PTAL. Thanks!

anurag629 · 2021-11-30T08:13:34Z

@vojtechjelinek Please help why checks are failing

vojtechjelinek · 2022-01-12T13:35:56Z

@anurag629 Please, do not resolve comments yourself, just reply with "Done" and then let the reviewer resolve it.

nithinrdy · 2022-01-13T16:05:42Z

Hey @anurag629, a PR was recently merged that fixes a flake in the frontend tests (the same flake that is causing the frontend tests to fail on this PR). Please update the PR with the latest changes in develop. Thanks!

anurag629 · 2022-01-14T08:31:53Z

@vojtechjelinek from where I will get these extra elements. I searched in the codebase but I am unable to find these.
What should I do ??

vojtechjelinek · 2022-01-15T14:13:22Z

@vojtechjelinek from where I will get these extra elements. I searched in the codebase but I am unable to find these. What should I do ??

This says that you need to remove these two from the constant at handler_schema_constants.py

…o anuragoppia2

anurag629 · 2022-01-16T03:38:52Z

@vojtechjelinek why the "End-to-End" test is failing. Previous they are not failing ???

vojtechjelinek · 2022-01-16T14:53:58Z

@vojtechjelinek why the "End-to-End" test is failing. Previous they are not failing ???

They are sometimes flaky, I have restarted them.

vojtechjelinek

Thanks! Took a pass.

vojtechjelinek · 2022-01-16T15:11:10Z

core/controllers/reader.py

+            'old_params': {
+                'schema': {
+                    'type': 'int',
+                }
+            },


Why is this int?

vojtechjelinek · 2022-01-16T15:11:27Z

core/controllers/reader.py

+            'client_time_spent_in_secs': {
+                'schema': {
+                    'type': 'int',
+                }
+            },


Should this have a minimal value of 0?

anurag629 · 2022-02-08T04:18:27Z

Thanks! Took a pass.

hey, @vojtechjelinek I was not active in the past days. Can you help me in resolving errors? Thanks!

vojtechjelinek · 2022-02-08T11:10:17Z

@anurag629 Can you address my comments first?

oppiabot · 2022-02-15T11:48:00Z

Hi @anurag629, 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 4 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 4 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

srijanreddy98 and others added 3 commits November 26, 2021 23:19

added first schema

3f9b06c

Merge branch 'oppia:develop' into anuragoppia2

533eb73

anurag629 requested a review from aks681 as a code owner November 27, 2021 19:56

oppiabot bot assigned anurag629 Nov 27, 2021

oppiabot bot added the REVIEWERS: Please add changelog label label Nov 27, 2021

oppiabot bot assigned aks681 Nov 27, 2021

anurag629 changed the title ~~Anuragoppia2~~ Fix part of #13162: StateHitEventHandler Nov 28, 2021

vojtechjelinek added PR CHANGELOG: Data handling -- @vojtechjelinek and removed REVIEWERS: Please add changelog label labels Nov 28, 2021

anurag629 added 3 commits November 29, 2021 00:46

Merge branch 'oppia:develop' into anuragoppia2

ba82eeb

Merge branch 'oppia:develop' into anuragoppia2

2ade3f6

removed handler from handler schema constant

b6ec9a5

anurag629 requested a review from vojtechjelinek as a code owner November 29, 2021 07:46

vojtechjelinek removed the REVIEWERS: Please add changelog label label Nov 29, 2021

vojtechjelinek requested changes Nov 29, 2021

View reviewed changes

vojtechjelinek unassigned anurag629 Nov 29, 2021

oppiabot bot assigned anurag629 Nov 29, 2021

anurag629 added 2 commits November 29, 2021 22:57

added arguments of ppost

3b4e3ab

Merge branch 'oppia:develop' into anuragoppia2

514938a

oppiabot bot added the PR: don't merge - STALE BUILD The build on this PR is stale and should be restarted. label Dec 1, 2021

anurag629 added 2 commits January 12, 2022 19:30

fixed lint check error

22f3216

again fixed lint checks

bf3a451

Merge branch 'oppia:develop' into anuragoppia2

527f6fb

Merge branch 'oppia:develop' into anuragoppia2

7f8ab8b

Your Name and others added 3 commits January 15, 2022 20:31

removed handlers from list which are done

0748b0b

Merge branch 'anuragoppia2' of https://github.com/anurag629/oppia int…

684d326

…o anuragoppia2

Merge branch 'oppia:develop' into anuragoppia2

f11a561

Merge branch 'oppia:develop' into anuragoppia2

db171ae

vojtechjelinek requested changes Jan 16, 2022

View reviewed changes

anurag629 added 10 commits January 17, 2022 22:22

Merge branch 'oppia:develop' into anuragoppia2

1a6958c

Merge branch 'oppia:develop' into anuragoppia2

9e7fa21

Merge branch 'oppia:develop' into anuragoppia2

ea5b230

Merge branch 'oppia:develop' into anuragoppia2

e444c89

Merge branch 'oppia:develop' into anuragoppia2

1212269

Merge branch 'oppia:develop' into anuragoppia2

1d6920f

Merge branch 'oppia:develop' into anuragoppia2

d597e78

Merge branch 'oppia:develop' into anuragoppia2

7c8bf8c

Merge branch 'oppia:develop' into anuragoppia2

a9049c6

Merge branch 'oppia:develop' into anuragoppia2

291c127

oppiabot bot added the stale label Feb 15, 2022

oppiabot bot closed this Feb 19, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix part of #13162: StateHitEventHandler #14326

Fix part of #13162: StateHitEventHandler #14326

anurag629 commented Nov 27, 2021 •

edited by vojtechjelinek

Loading

oppiabot bot commented Nov 27, 2021

oppiabot bot commented Nov 27, 2021

anurag629 commented Nov 27, 2021

vojtechjelinek commented Nov 28, 2021

vojtechjelinek commented Nov 29, 2021

anurag629 commented Nov 29, 2021

vojtechjelinek left a comment

vojtechjelinek Nov 29, 2021

anurag629 Jan 13, 2022

vojtechjelinek Nov 29, 2021

anurag629 Jan 13, 2022

oppiabot bot commented Nov 29, 2021

anurag629 commented Nov 30, 2021

vojtechjelinek commented Jan 12, 2022

nithinrdy commented Jan 13, 2022

anurag629 commented Jan 14, 2022

vojtechjelinek commented Jan 15, 2022

anurag629 commented Jan 16, 2022

vojtechjelinek commented Jan 16, 2022

vojtechjelinek left a comment

vojtechjelinek Jan 16, 2022

vojtechjelinek Jan 16, 2022

anurag629 commented Feb 8, 2022 •

edited

Loading

vojtechjelinek commented Feb 8, 2022

oppiabot bot commented Feb 15, 2022

Fix part of #13162: StateHitEventHandler #14326

Fix part of #13162: StateHitEventHandler #14326

Conversation

anurag629 commented Nov 27, 2021 • edited by vojtechjelinek Loading

Overview

Essential Checklist

Proof that changes are correct

PR Pointers

oppiabot bot commented Nov 27, 2021

oppiabot bot commented Nov 27, 2021

anurag629 commented Nov 27, 2021

vojtechjelinek commented Nov 28, 2021

vojtechjelinek commented Nov 29, 2021

anurag629 commented Nov 29, 2021

vojtechjelinek left a comment

Choose a reason for hiding this comment

vojtechjelinek Nov 29, 2021

Choose a reason for hiding this comment

anurag629 Jan 13, 2022

Choose a reason for hiding this comment

vojtechjelinek Nov 29, 2021

Choose a reason for hiding this comment

anurag629 Jan 13, 2022

Choose a reason for hiding this comment

oppiabot bot commented Nov 29, 2021

anurag629 commented Nov 30, 2021

vojtechjelinek commented Jan 12, 2022

nithinrdy commented Jan 13, 2022

anurag629 commented Jan 14, 2022

vojtechjelinek commented Jan 15, 2022

anurag629 commented Jan 16, 2022

vojtechjelinek commented Jan 16, 2022

vojtechjelinek left a comment

Choose a reason for hiding this comment

vojtechjelinek Jan 16, 2022

Choose a reason for hiding this comment

vojtechjelinek Jan 16, 2022

Choose a reason for hiding this comment

anurag629 commented Feb 8, 2022 • edited Loading

vojtechjelinek commented Feb 8, 2022

oppiabot bot commented Feb 15, 2022

anurag629 commented Nov 27, 2021 •

edited by vojtechjelinek

Loading

anurag629 commented Feb 8, 2022 •

edited

Loading