search

mitodl / rapid-response-xblock

a django app plug-in for edx-platform
BSD 3-Clause "New" or "Revised" License
0 stars 2 forks source link

Fixes: Page scrolls when closing rapid response problem #123

Closed asadiqbal08 closed 2 years ago

asadiqbal08 commented 2 years ago

What are the relevant tickets?

fixes: #122

What's this PR do?

Fix the page scroll issue as mentioned in the ticket.

How should this be manually tested?

Follow the Readme Steps to install the Rapid Response block in Studio and CMS. Verify that the page should not be scroll on either opening or closing the problem in LMS.

arslanashraf7 commented 2 years ago

@asadiqbal08 Can you look at the failing tests?

asadiqbal08 commented 2 years ago

@arslanashraf7 I am not sure why integration tests are failing over here. I looked around but the reasons of failing does not seem related to my changes.

As a TestCase, I reverted my changes in 2nd commit but the build is failing again.

@pdpinch do u have some thoughts about the pipeline flow here ?

asadiqbal08 commented 2 years ago

The ci actions are pointing to master of mitodl for edx-platform

arslanashraf7 commented 2 years ago

@asadiqbal08 FYI.

@arslanashraf7 I am not sure why integration tests are failing over here. I looked around but the reasons of failing does not seem related to my changes.

As a TestCase, I reverted my changes in 2nd commit but the build is failing again.

The initial look at the failing logs gives me an impression that something in the course structure has been changed. The tests are mostly failing in the setup part of our test suit and complaining about the index/course lookup being unavailable. I think we might have to look back a little to see the changes recently done in the courseware lookup in the edX.

Since the tests are failing and it's possible that our functionality might also be broken. So, while making this change did you test it and see that the xBlock is working fine?

The ci actions are pointing to master of mitodl for edx-platform

In our django upgrade PR I mentioned the reason for that here because our tests were failing unnecessarily. And I think it's time to take it back to mitx/maple. Can you try running the tests on https://github.com/mitodl/edx-platform/tree/mitx/maple and see if we get different results in the CI?

asadiqbal08 commented 2 years ago

@arslanashraf7 can you test the scroll fix here ?

asadiqbal08 commented 2 years ago

@pdpinch @arslanashraf7 Integration tests are failing due to some incompatibility with edx branches. I would suggest, We should open another issue to trace the failure because the fix in this PR does not causing the integration failure for edx. I updated the github CI actions and observed different outcomes.

In past, CI actions were introduced here and updated during Django upgrade Here.