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: use primary key for link lookup #36919

Merged
merged 1 commit into from
Sep 6, 2023

Conversation

dj12djdjs
Copy link
Collaborator

Overview

The stock balance report uses the tabItem.item_code column as a lookup for it's item filter. This differs from the expected name column used as lookup.

Context

image

The item has a name of 597f38b5c1 and an item_code of Demo Item

Before PR

image

After PR

image

@github-actions github-actions bot added the needs-tests This PR needs automated unit-tests. label Sep 2, 2023
@codecov
Copy link

codecov bot commented Sep 2, 2023

Codecov Report

Merging #36919 (a29f54d) into develop (0e51722) will increase coverage by 0.12%.
Report is 8 commits behind head on develop.
The diff coverage is 66.66%.

@@             Coverage Diff             @@
##           develop   #36919      +/-   ##
===========================================
+ Coverage    65.42%   65.54%   +0.12%     
===========================================
  Files          794      795       +1     
  Lines        62194    62642     +448     
===========================================
+ Hits         40690    41059     +369     
- Misses       21504    21583      +79     
Files Changed Coverage
...rpnext/stock/report/stock_balance/stock_balance.py 66.66%

@ankush
Copy link
Member

ankush commented Sep 3, 2023

When do item_code and name differ for item? They are supposed to be always same.

The change is okay, but just wondering.

@dj12djdjs
Copy link
Collaborator Author

@ankush In hindsight it is because of a flaw in my design for a customers customization.

The request was the make Item a two column lookup. Brand and Item Code, so these two columns are representing a unique record. Due to this customization, name becomes a random hash, and item_code becomes the sku.
You can see this customization in action here: frappe/frappe#21255

I should have have used item naming based on naming series, then used Brand, and Item Name as the unique record lookup. But I already made the customization. As far as I know this is the only function where my customization doesn't work.

@s-aga-r s-aga-r merged commit 8ce6b81 into frappe:develop Sep 6, 2023
1 check passed
@s-aga-r s-aga-r added the backport version-14-hotfix backport to version 14 label Sep 6, 2023
mergify bot pushed a commit that referenced this pull request Sep 6, 2023
s-aga-r pushed a commit that referenced this pull request Sep 6, 2023
fix: use primary key for link lookup (#36919)

(cherry picked from commit 8ce6b81)

Co-authored-by: Devin Slauenwhite <[email protected]>
frappe-pr-bot pushed a commit that referenced this pull request Sep 12, 2023
# [14.39.0](v14.38.0...v14.39.0) (2023-09-12)

### Bug Fixes

* `company` is ambiguous ([fe69d53](fe69d53))
* `Parent Task` link with `Project Task` (backport [#37025](#37025)) ([#37033](#37033)) ([6602787](6602787))
* correct asset daily depr schedule calculation [v14] ([#36991](#36991)) ([2ae4463](2ae4463))
* generate pdf only when result exists ([53270dd](53270dd))
* remove report field db set ([284181d](284181d))
* show letterhead and terms for AR pdf ([2077b2c](2077b2c))
* Update party type for payroll payable account ([f251d6c](f251d6c))
* use primary key for link lookup (backport [#36919](#36919)) ([#36978](#36978)) ([4fede56](4fede56))
* **ux:** docstatus filter for `Reference Name` in QI (backport [#37024](#37024)) ([#37028](#37028)) ([21be889](21be889))

### Features

* add field for specifying pdf name ([657ca7f](657ca7f))
* Add half-yearly asset maintenance periodicity. (backport [#37006](#37006)) ([#37014](#37014)) ([acd9c69](acd9c69))
* provision to set required by from Production Plan ([#37039](#37039)) ([d278b11](d278b11))
@dj12djdjs dj12djdjs deleted the fix-stock-balance-item-filter branch September 14, 2023 17:23
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport version-14-hotfix backport to version 14 needs-tests This PR needs automated unit-tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants