Codecov Report

Merging #2389 (4101bb7) into master (b1da2fc) will increase coverage by 8.68%.
The diff coverage is n/a.

❗ Current head 4101bb7 differs from pull request most recent head c1b836d. Consider uploading reports for the commit c1b836d to get more accurate results

@@            Coverage Diff             @@
##           master    #2389      +/-   ##
==========================================
+ Coverage   88.29%   96.98%   +8.68%     
==========================================
  Files         330      156     -174     
  Lines       15102     4979   -10123     
  Branches     1704        0    -1704     
==========================================
- Hits        13335     4829    -8506     
+ Misses       1517      150    -1367     
+ Partials      250        0     -250     

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b1da2fc...c1b836d. Read the comment docs.

Today, I noticed an issue after moving the data-migration logic from migration file to manage command. After running some uniittests, it seems to me they depends upon migrate_data logic that was earlier in migration file. I am not sure, maybe that was the reason earlier to keep this stuff in migration files instead of moving under a management command.
So most of the unittests are failing due to this reason.

unittests expect to have page structures as per the logic and failed.

@arslanashraf7 do you think can we proceed with this PR or need to close it. ?

cc @pdpinch

@arslanashraf7 do you think can we proceed with this PR or need to close it. ?

cc @pdpinch

As per your comment i think of these options:

• With your assessment do you think either running this command as is or converting it into a script that runs before our test step runs in the CI would help fix the tests?
• If the above isn't feasible, and that might be the case since not only tests but the actual database gets updated when these migrations run on an existing instance like RC, we might end up breaking a few things that might take more time to fix later. We should definitely close the PR in this case.

I'll go with your assessment.