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(ux)!: when error messages are present, supress non-error messages #16284

Merged
merged 2 commits into from
Mar 15, 2022

Conversation

ankush
Copy link
Member

@ankush ankush commented Mar 15, 2022

Problem: During network request handling we often print several messages to help users... however during the course of handling such requests a single frappe.throw usually means full rollback of everything that happened in the request. Hence previous messages make no sense.

Summary of change: if there are any messages with raise_exception then only show errors.

Minimal reproducible example:

def onload(self):
    frappe.msgprint("success")
    frappe.throw("failure")

This will show up as:
Screenshot 2022-03-15 at 1 52 48 PM

Another real example when serial no generation fails:

Screenshot 2022-03-15 at 2 49 47 PM

XP vibes.

EEXPEEE

After this change:
Screenshot 2022-03-15 at 2 31 26 PM

Screenshot 2022-03-15 at 2 49 58 PM

closes #15674

@ankush ankush marked this pull request as ready for review March 15, 2022 09:32
@ankush ankush requested review from a team and surajshetty3416 and removed request for a team March 15, 2022 09:32
@codecov
Copy link

codecov bot commented Mar 15, 2022

Codecov Report

Merging #16284 (55e2480) into develop (b2e1155) will decrease coverage by 3.05%.
The diff coverage is 85.71%.

❗ Current head 55e2480 differs from pull request most recent head f8673a0. Consider uploading reports for the commit f8673a0 to get more accurate results

@@             Coverage Diff             @@
##           develop   #16284      +/-   ##
===========================================
- Coverage    56.76%   53.71%   -3.06%     
===========================================
  Files          756      752       -4     
  Lines        66827    66482     -345     
  Branches      5696     5616      -80     
===========================================
- Hits         37936    35709    -2227     
- Misses       25449    26771    +1322     
- Partials      3442     4002     +560     
Flag Coverage Δ
ui-tests 38.35% <85.71%> (-8.81%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@ankush
Copy link
Member Author

ankush commented Mar 15, 2022

image

unrelated test failure in list view paging. Test helper function timed out.

@ankush ankush changed the title fix(ux)!: when error messages are present supress non-error messages fix(ux)!: when error messages are present, supress non-error messages Mar 15, 2022
@ankush ankush merged commit e0def6f into frappe:develop Mar 15, 2022
@ankush ankush deleted the misleading_msgprint branch March 15, 2022 10:21
ankush added a commit to frappe/erpnext that referenced this pull request Mar 16, 2022
noahjacob pushed a commit to noahjacob/erpnext that referenced this pull request Mar 21, 2022
KrithiRamani pushed a commit to KrithiRamani/erpnext that referenced this pull request Mar 28, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Misleading Serial no. creation pop up during purchase receipt submission
2 participants