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.

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

fix: Fix rows counter in the Edit Grade modal window #310

Conversation

Lunyachek
Copy link
Contributor

TL;DR - The problem was in the rows counter in the Edit Grades modal window. First digit - number of lines excluding the last line with the form. Second digit - grades data. And our proposal is to include last row with form to common counting

Снимок экрана 2023-02-10 в 15 25 06

What changed?

Снимок экрана 2023-02-10 в 15 42 01

FYI: @openedx/content-aurora

@Lunyachek Lunyachek requested a review from a team as a code owner February 10, 2023 13:44
@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Feb 10, 2023
@openedx-webhooks
Copy link

openedx-webhooks commented Feb 10, 2023

Thanks for the pull request, @Lunyachek! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

@codecov
Copy link

codecov bot commented Feb 14, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (ef8e20f) to head (494eabc).
Report is 2 commits behind head on master.

❗ Current head 494eabc differs from pull request most recent head 18cede4. Consider uploading reports for the commit 18cede4 to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           master      #310      +/-   ##
===========================================
+ Coverage   94.98%   100.00%   +5.01%     
===========================================
  Files         139       109      -30     
  Lines        1355      1265      -90     
  Branches      264       248      -16     
===========================================
- Hits         1287      1265      -22     
+ Misses         60         0      -60     
+ Partials        8         0       -8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mphilbrick211
Copy link

@jmakowski1123 @ProductRyan flagging this as needing product review to move forward. Thanks!

@jmakowski1123
Copy link

Since this fix has multiple PRs associated with it (#310 and #311), I've created a Product Feature ticket. It's currently undergoing Product Review in that Feature Ticket.

@Daniel-hershel
Copy link

Hey, I approved this on the product ticket noted above

@mphilbrick211
Copy link

Hi @mattcarter @openedx/content-aurora! This is ready for your review. If all looks good, please merge as well. Thank you!

@mphilbrick211
Copy link

Hi @mattcarter @openedx/content-aurora! This is ready for your review. If all looks good, please merge as well. Thank you!

Hi @mattcarter - friendly ping on this :)

@mphilbrick211
Copy link

Hi @mattcarter and @openedx/content-aurora! Would someone be able to review/merge this? Thanks!

In the meantime, @Lunyachek - could you please take a look at resolving the branch conflicts?

@mphilbrick211
Copy link

Hi @Lunyachek - just checking to see if you're able to resolve the conflicts here? Thanks!

@mphilbrick211
Copy link

Hi @Lunyachek! Friendly follow-up on this!

@mphilbrick211
Copy link

Hi @Lunyachek! Just checking in to see if you plan to pursue this PR?

@mphilbrick211 mphilbrick211 added the inactive PR author has been unresponsive for several months label Oct 3, 2023
@jmakowski1123
Copy link

This one has product approval: openedx/platform-roadmap#236 (comment)

@mphilbrick211
Copy link

Hi @Lunyachek - closing this for now. In order to pursue, we need the branch conflicts resolved. If you'd like to reopen, please let us know.

@openedx-webhooks
Copy link

@Lunyachek Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.

@mphilbrick211 mphilbrick211 added closed inactivity PR was closed because the author abandoned it and removed inactive PR author has been unresponsive for several months labels Jan 16, 2024
@Lunyachek
Copy link
Contributor Author

@mphilbrick211 Hello. Yes, I want to reopen this PR. I also made a backport to the quince branch - #389

@mphilbrick211 mphilbrick211 reopened this Apr 5, 2024
@mphilbrick211 mphilbrick211 removed the request for review from a team April 5, 2024 00:51
@mphilbrick211 mphilbrick211 added product review complete PR has gone through product review and removed product review PR requires product review before merging labels Apr 5, 2024
@mphilbrick211
Copy link

Hi @Lunyachek! I've reopened. Would you mind resolving the branch conflicts? Thanks!

@Lunyachek Lunyachek force-pushed the lunyachek/fix/edit-grades-modal-rows-counter-olive branch from 494eabc to 18cede4 Compare April 18, 2024 20:36
@Lunyachek
Copy link
Contributor Author

Hi @Lunyachek! I've reopened. Would you mind resolving the branch conflicts? Thanks!

Thank you! Conflicts resolved

@mphilbrick211 mphilbrick211 removed the closed inactivity PR was closed because the author abandoned it label Apr 22, 2024
@mphilbrick211
Copy link

Hi @arbrandes! Would you mind taking a look at this?

Copy link
Contributor

@arbrandes arbrandes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@arbrandes arbrandes merged commit 13433e9 into openedx:master May 30, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
open-source-contribution PR author is not from Axim or 2U product review complete PR has gone through product review
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants