oppia icon indicating copy to clipboard operation
oppia copied to clipboard

Fix part of issue #19435: Migrate the Review Test page

Open Ahmed-Bhouri opened this issue 10 months ago • 18 comments

Overview

  1. This PR fixes or fixes part of #19435.
  2. This PR does the following: Migrates the Review Test page from Webpack/AngularJS to Lazy-Loaded Angular Module

Essential Checklist

Please follow the instructions for making a code change.

  • [ ] I have linked the issue that this PR fixes in the "Development" section of the sidebar.
  • [x] I have checked the "Files Changed" tab and confirmed that the changes are what I want to make.
  • [x] I have written tests for my code.
  • [x] The PR title starts with "Fix #bugnum: " or "Fix part of #bugnum: ...", followed by a short, clear summary of the changes.
  • [x] I have assigned the correct reviewers to this PR (or will leave a comment with the phrase "@{{reviewer_username}} PTAL" if I can't assign them directly).

Proof that changes are correct

https://github.com/oppia/oppia/assets/49922043/e4eb0570-39fe-448a-bec1-93da4acfcbc5

PR Pointers

  • Never force push! If you do, your PR will be closed.
  • To reply to reviewers, follow these instructions: https://github.com/oppia/oppia/wiki/Make-a-pull-request#step-5-address-review-comments-until-all-reviewers-approve
  • Some 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.
  • See the Code Owner's wiki page for what code owners will expect.

Ahmed-Bhouri avatar Apr 01 '24 20:04 Ahmed-Bhouri

Hi @Ahmed-Bhouri please assign the required reviewer(s) for this PR. Thanks!

oppiabot[bot] avatar Apr 01 '24 20:04 oppiabot[bot]

@DubeySandeep PTAL

Ahmed-Bhouri avatar Apr 01 '24 20:04 Ahmed-Bhouri

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

oppiabot[bot] avatar Apr 01 '24 20:04 oppiabot[bot]

Unassigning @DubeySandeep since the review is done.

oppiabot[bot] avatar Apr 02 '24 08:04 oppiabot[bot]

Hi @Ahmed-Bhouri, it looks like some changes were requested on this pull request by @DubeySandeep. PTAL. Thanks!

oppiabot[bot] avatar Apr 02 '24 08:04 oppiabot[bot]

@DubeySandeep Thank you for the review, I tried to address the issues mentioned. I think I can change URL to a query string if necessary but It was the used practice and I think it will be better to keep the code homogenous. I will be changing it to a PR in a sec. PTAL

Ahmed-Bhouri avatar Apr 02 '24 08:04 Ahmed-Bhouri

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

oppiabot[bot] avatar Apr 02 '24 08:04 oppiabot[bot]

@seanlip PTAL to my reply on the review and should I change it?

Ahmed-Bhouri avatar Apr 07 '24 14:04 Ahmed-Bhouri

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

oppiabot[bot] avatar Apr 07 '24 14:04 oppiabot[bot]

@seanlip PTAL

Ahmed-Bhouri avatar Apr 12 '24 08:04 Ahmed-Bhouri

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

oppiabot[bot] avatar Apr 12 '24 08:04 oppiabot[bot]

@seanlip PTAL

Ahmed-Bhouri avatar Apr 15 '24 14:04 Ahmed-Bhouri

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

oppiabot[bot] avatar Apr 15 '24 14:04 oppiabot[bot]

@Ahmed-Bhouri I think I left a reply just before you mentioned me, PTAL. Thanks!

seanlip avatar Apr 15 '24 15:04 seanlip

Hi @Ahmed-Bhouri. Due to recent changes in the "develop" branch, this PR now has a merge conflict. Please follow this link if you need help resolving the conflict, so that the PR can be merged. Thanks!

oppiabot[bot] avatar Apr 19 '24 16:04 oppiabot[bot]

@kevintab95 Can you please take a pass on this PR?

DubeySandeep avatar Apr 25 '24 07:04 DubeySandeep

Hi @Ahmed-Bhouri. Due to recent changes in the "develop" branch, this PR now has a merge conflict. Please follow this link if you need help resolving the conflict, so that the PR can be merged. Thanks!

oppiabot[bot] avatar Apr 28 '24 19:04 oppiabot[bot]

Hi @Ahmed-Bhouri, 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!

oppiabot[bot] avatar May 05 '24 19:05 oppiabot[bot]