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: Quality Inspection Rejection issue fixed #606

Merged
merged 11 commits into from
Oct 13, 2020
Merged

fix: Quality Inspection Rejection issue fixed #606

merged 11 commits into from
Oct 13, 2020

Conversation

bhattdevarsh
Copy link

TASK

Reported Isssue
PR Reference

  1. If Reading Parameter set as "Rejected" and clicked "Yes" in Confirm box > QI status will be set as Rejected
  2. If Reading Parameter set as "Rejected" and clicked "No" in Confirm box > QI status will remain same and Reading Parameter status will Rollback.
  3. If ONE Reading Parameter set as "Rejected" and later changed to "Accepted" > QI status will also set as "Accepted"
  4. If MULTIPLE Reading Parameter set as "Rejected" and ONE Parameter Changed to "Accepted" > QI status will remain "Rejected"

QI_error_msg

@coveralls
Copy link

coveralls commented Sep 28, 2020

Pull Request Test Coverage Report for Build 2397

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 0.0%

Totals Coverage Status
Change from base Build 2394: 0.0%
Covered Lines: 0
Relevant Lines: 0

💛 - Coveralls

@@ -63,12 +63,23 @@ frappe.ui.form.on("Quality Inspection", {
frappe.ui.form.on("Quality Inspection Reading", {
status: function (frm, cdt, cdn) {
let row = locals[cdt][cdn];
let not_rejected = true;
Copy link

Choose a reason for hiding this comment

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

Couldn't this be:

Suggested change
let not_rejected = true;
let rejected = false;

Seems like that'll be more readable

Comment on lines 73 to 82
else {
frm.doc.readings.forEach(reading => {
if (reading.status === "Rejected") {
not_rejected = false;
}
});
if (not_rejected) {
frm.set_value("status", "Accepted");
}
}
Copy link

Choose a reason for hiding this comment

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

This should happen on the server side as well

@@ -68,10 +68,14 @@ def validate_certificate_of_analysis(self):
frappe.throw(_("Please attach a Certificate of Analysis"))

def validate_reading_status(self):
rejected = False
Copy link

Choose a reason for hiding this comment

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

Suggested change
rejected = False
self.status = "Accepted"

for reading in self.readings:
if reading.status == 'Rejected':
self.status = "Rejected"
return
rejected = True
Copy link

Choose a reason for hiding this comment

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

Suggested change
rejected = True

Comment on lines 77 to 78
if not rejected:
self.status = "Accepted"
Copy link

Choose a reason for hiding this comment

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

Suggested change
if not rejected:
self.status = "Accepted"

@bhattdevarsh bhattdevarsh requested a review from hrwX October 5, 2020 07:01
Comment on lines 73 to 82
else {
frm.doc.readings.forEach(reading => {
if (reading.status === "Rejected") {
rejected = true;
}
});
if (!rejected) {
frm.set_value("status", "Accepted");
}
}
Copy link

Choose a reason for hiding this comment

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

Suggested change
else {
frm.doc.readings.forEach(reading => {
if (reading.status === "Rejected") {
rejected = true;
}
});
if (!rejected) {
frm.set_value("status", "Accepted");
}
}
let status = "Accepted";
frm.doc.readings.forEach(reading => {
if (reading.status === "Rejected") {
status = "Rejected";
}
});
frm.set_value("status", status);

Co-authored-by: Himanshu <himanshuwarekar@yahoo.com>
@hrwX hrwX changed the base branch from staging to develop October 13, 2020 06:51
@hrwX hrwX merged commit 1e81806 into Bloomstack:develop Oct 13, 2020
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.

4 participants