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: Sales Order cancellation message #27735

Merged
merged 17 commits into from
Nov 15, 2021

Conversation

GangaManoj
Copy link
Contributor

@GangaManoj GangaManoj commented Oct 1, 2021

Problem:

When cancelling a Sales Order which has a Sales Invoice in the draft state linked to it:

  • The user is directed to 'cancel' the linked invoice, even though only submitted invoices can be cancelled.
  • If the invoice has more than one Item, the name of the invoice will be repeated multiple times(as many times as there are Items in it).

Screenshot 2021-10-02 at 2 21 33 AM

Fix:

  • Edit the message to instruct the user to delete the linked invoice instead.
  • Only display the name of the invoice once, irrespective of the number of Items in it.

Additional changes:

  • Remove redundant code
  • Add tests

@codecov
Copy link

codecov bot commented Oct 1, 2021

Codecov Report

Merging #27735 (cc07c71) into develop (f24ed67) will increase coverage by 0.09%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop   #27735      +/-   ##
===========================================
+ Coverage    55.10%   55.20%   +0.09%     
===========================================
  Files         1255     1255              
  Lines        67929    67913      -16     
===========================================
+ Hits         37431    37488      +57     
+ Misses       30498    30425      -73     
Impacted Files Coverage Δ
erpnext/selling/doctype/sales_order/sales_order.py 77.58% <100.00%> (+1.19%) ⬆️
erpnext/education/doctype/student/student.py 73.95% <0.00%> (-3.13%) ⬇️
...value/warehouse_wise_item_balance_age_and_value.py 92.59% <0.00%> (-2.47%) ⬇️
erpnext/stock/stock_ledger.py 85.32% <0.00%> (-2.00%) ⬇️
...ion/doctype/course_enrollment/course_enrollment.py 45.09% <0.00%> (-1.97%) ⬇️
erpnext/portal/utils.py 30.00% <0.00%> (-1.43%) ⬇️
erpnext/stock/reorder_item.py 76.27% <0.00%> (-0.85%) ⬇️
...ype/account/chart_of_accounts/chart_of_accounts.py 77.55% <0.00%> (-0.69%) ⬇️
...next/accounts/doctype/subscription/subscription.py 82.02% <0.00%> (-0.57%) ⬇️
erpnext/stock/get_item_details.py 79.51% <0.00%> (-0.46%) ⬇️
... and 19 more

@stale
Copy link

stale bot commented Oct 20, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed within a week if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing.

@stale stale bot added the inactive label Oct 20, 2021
@stale stale bot closed this Oct 23, 2021
@GangaManoj GangaManoj reopened this Oct 24, 2021
@stale stale bot removed the inactive label Oct 24, 2021
@stale
Copy link

stale bot commented Nov 11, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed within a week if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing.

@stale stale bot added the inactive label Nov 11, 2021
@deepeshgarg007 deepeshgarg007 merged commit ce06aaa into frappe:develop Nov 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants