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.

Sign up for GitHub

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

Jump to bottom

fix: Don't create inward SLE against SI unless is internal customer enabled #27086

Merged
merged 3 commits into from
Aug 26, 2021
Merged

fix: Don't create inward SLE against SI unless is internal customer enabled #27086

merged 3 commits into from
Aug 26, 2021

Conversation

marination
Copy link
Collaborator

@marination marination commented Aug 23, 2021 •
edited
Loading

Issue:

  • Due to duplication/data import Target Warehouse could be copied onto multiple Sales Invoices from one Sales Invoice
  • If target warehouse exists accidentally, and the customer is not actually an internal customer, inward SLEs were created against such Sales Invoices

Fix:

  • Check for internal customer along with target warehouse before making SLEs
  • Test to check if inward SLE is made if target warehouse is accidentally set but customer is not internal

In this case a user messed around with property setter and disabled no copy on this hidden target warehouse field in Sales Invoice. no_copy is enabled otherwise. This is a defensive fix. Either way the check should be stricter before making something as irreversible as SLEs.

Another Note: It seems like before the introduction of internal customer, target warehouse inward entries were the norm if target warehouse exists (a test exists for this behaviour). Now since target warehouse is hidden if not internal customer this behaviour is discontinued.

@marination
fix: Dont create inward SLE against SI unless is internal customer en…
656d796 
…abled

- Check if is internal customer enabled apart from target warehouse
- Test to check if inward SLE is made if target warehouse is accidentally set but customer is not internal
@marination marination added the CI-failing Unit tests or patch tests are failing. label Aug 24, 2021
@deepeshgarg007 deepeshgarg007 self-assigned this Aug 25, 2021
@marination
Merge branch 'develop' into target-warehouse-value
fd863e6
@marination
test: Use internal customer for delivery of bundle items to target wa…
447714a 
…rehouse

- created `create_internal_customer` util
- reused it in delivery note and sales invoice tests
- use internal customer for target warehouse test in delivery note
@marination marination requested a review from ankush as a code owner August 26, 2021 07:19
@marination
Copy link
Collaborator Author

test_closing_entry failing

@marination marination added backport version-13-hotfix and removed CI-failing Unit tests or patch tests are failing. labels Aug 26, 2021
@marination marination merged commit f4dc9ee into frappe:develop Aug 26, 2021
frappe-pr-bot pushed a commit to frappe-pr-bot/erpnext that referenced this pull request Aug 26, 2021
@marination @frappe-bot
fix: Don't create inward SLE against SI unless is internal customer e…
6636d85 
…nabled (frappe#27086)

* fix: Dont create inward SLE against SI unless is internal customer enabled

- Check if is internal customer enabled apart from target warehouse
- Test to check if inward SLE is made if target warehouse is accidentally set but customer is not internal

* test: Use internal customer for delivery of bundle items to target warehouse

- created `create_internal_customer` util
- reused it in delivery note and sales invoice tests
- use internal customer for target warehouse test in delivery note

(cherry picked from commit f4dc9ee)

# Conflicts:
#	erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py
@frappe-pr-bot frappe-pr-bot mentioned this pull request Aug 26, 2021
Merged
marination added a commit to marination/erpnext that referenced this pull request Aug 26, 2021
@marination
fix: Remove duplicate create_internal_customer util
45edfd9 
- Introduced via frappe#27086
@marination marination mentioned this pull request Aug 26, 2021
Merged
nabinhait added a commit that referenced this pull request Aug 26, 2021
@rohitwaghchaure @18alantom
@marination @nabinhait @nextchamp-saqib @frappe-pr-bot @ankush @ruchamahabal
chore: merge branch 'version-13-hotfix' into 'version-13-pre-release' (…
7c31e1f 
…#27173)

* feat: add provision for process loss in manufac

* feat: add is process loss autoset and validation

* fix: add warehouse and unset is scrap for process loss items

* refactor: shift auto entry of is process loss check, update validations

* test: add bom tests for process loss val, add se test for qty calc

* fix: add more validations, remove source wh req for pl item

* fix: sider

* refactor: polyfill ??

* fix: sider

* refactor: validation error message formatting

* test: check manufacture completion qty in se and wo

* fix: wo tests, sider, account for pl in se validation

* fix: reword error messages, fix test values

* feat: add procss_loss_qty field in work order

* feat: process loss report, fix set pl query condition

* fix: correct value in test

* fix: get filters to work
- reorder and rename columns
- add work order filter

* fix: Shopping cart Exchange rate validation (#27050)

* fix: Shopping cart Exchange rate validation

- Use `get_exchange_rate` to check for price list exchange rate in cart settings
- Move cart exchange rate validation for Price List from hooks to doc event
- Call cart exchange rate validation on PL update only if PL is in cart and currency is changed

* chore: Comment out obsolete test

- Modifying this test means considering extreme edge cases, which seems pointless now

* fix: Remove snippet that got in due to cherry-pick from `develop`

- This snippet is not present in v13-hotfix. Via #26520

Co-authored-by: Nabin Hait <nabinhait@gmail.com>

* feat: initialize party link for customer & suppliers

* feat: toggle to enable common party accounting

* feat: auto create advance entry on invoice submission

* test: creation of advance entry on invoice submission

* fix: remove unwanted filter query

* feat: validate multiple links

* fix: party link permissions

* perf: reduce number of queries to get party link

* fix: cost center & naming series

* fix: cost center in test_sales_invoice_against_supplier

* fix: Don't create inward SLE against SI unless is internal customer enabled (#27086)

* fix: Dont create inward SLE against SI unless is internal customer enabled

- Check if is internal customer enabled apart from target warehouse
- Test to check if inward SLE is made if target warehouse is accidentally set but customer is not internal

* test: Use internal customer for delivery of bundle items to target warehouse

- created `create_internal_customer` util
- reused it in delivery note and sales invoice tests
- use internal customer for target warehouse test in delivery note

(cherry picked from commit f4dc9ee)

# Conflicts:
#	erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py

* fix: prevent over riding scrap table values, name kwargs, set currency

* fix(regional): minor fixes and test for South Africa VAT report (#26933) (#27162)

* fix: allow to change incoming rate manually in case of stand-alone credit note (#27164)

* fix: allow to change rate manually in case of stand-alone credit note (#27036)

Co-authored-by: Marica <maricadsouza221197@gmail.com>
(cherry picked from commit fe4540d)

# Conflicts:
#	erpnext/accounts/doctype/sales_invoice_item/sales_invoice_item.json

* fix: resolve conflicts

Co-authored-by: rohitwaghchaure <rohitw1991@gmail.com>
Co-authored-by: Ankush Menat <ankush@iwebnotes.com>

* fix: Fee Validity fixes (#27161)

* fix: Fee Validity fixes (#27156)

* chore: update Fee Validity form labels

* fix: first appointment should not be considered for Fee Validity

* fix: Fee Validity test cases

* fix: appointment test case

(cherry picked from commit 642b4c8)

* fix: overlapping appointments

Co-authored-by: Rucha Mahabal <ruchamahabal2@gmail.com>

* fix: Merge conflicts and place internal customer creation util in test_customer.py

* fix: internal customer util returns 'str' not doc object

* fix: negative qty validation on stock reco cancellation (#27170) (#27171)

* test: negative stock validation on SR cancel

* fix: negative stock setting ignored in stock reco

In stock reconcilation cancellation negative stock setting is ignored as
`db.get_value` is returning string `'0'` which is not casted to int/bool
for further logic. This causes negative qty, which evantually gets
caught by reposting but by design this should stop cancellation.

* test: typo and minor refactor

(cherry picked from commit e7109c1)

Co-authored-by: Ankush Menat <ankush@iwebnotes.com>

Co-authored-by: 18alantom <2.alan.tom@gmail.com>
Co-authored-by: Marica <maricadsouza221197@gmail.com>
Co-authored-by: Nabin Hait <nabinhait@gmail.com>
Co-authored-by: Saqib Ansari <nextchamp.saqib@gmail.com>
Co-authored-by: Frappe PR Bot <frappe.pr.bot@gmail.com>
Co-authored-by: Ankush Menat <ankush@iwebnotes.com>
Co-authored-by: Rucha Mahabal <ruchamahabal2@gmail.com>
@rohitwaghchaure rohitwaghchaure mentioned this pull request Sep 15, 2021
Merged
asoral pushed a commit to asoral/erpnext that referenced this pull request Nov 12, 2021
@marination
fix: Don't create inward SLE against SI unless is internal customer e…
2668db2 
…nabled (frappe#27086)

* fix: Dont create inward SLE against SI unless is internal customer enabled

- Check if is internal customer enabled apart from target warehouse
- Test to check if inward SLE is made if target warehouse is accidentally set but customer is not internal

* test: Use internal customer for delivery of bundle items to target warehouse

- created `create_internal_customer` util
- reused it in delivery note and sales invoice tests
- use internal customer for target warehouse test in delivery note
asoral pushed a commit to asoral/erpnext that referenced this pull request Nov 12, 2021
@marination
fix: Remove duplicate create_internal_customer util
d580975 
- Introduced via frappe#27086
@marination marination mentioned this pull request Nov 25, 2021
Merged
@mergify mergify bot mentioned this pull request Feb 1, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@deepeshgarg007 deepeshgarg007 Awaiting requested review from deepeshgarg007

@nextchamp-saqib nextchamp-saqib Awaiting requested review from nextchamp-saqib

@rohitwaghchaure rohitwaghchaure Awaiting requested review from rohitwaghchaure rohitwaghchaure is a code owner

@ankush ankush Awaiting requested review from ankush

Assignees

@deepeshgarg007 deepeshgarg007

Labels
backport version-13-hotfix
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@marination @deepeshgarg007