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: manual depr entry not updating asset value [v14] #33788

Merged
merged 22 commits into from
Jan 31, 2023
Merged

fix: manual depr entry not updating asset value [v14] #33788

merged 22 commits into from
Jan 31, 2023

Conversation

anandbaburajan
Copy link
Contributor

@anandbaburajan anandbaburajan commented Jan 23, 2023 •
edited
Loading

  1. When manual depreciation entries (created using the Create Depreciation Entry option in the Manage dropdown of assets with calculate_depreciation not checked) are submitted, the value of the asset isn't updated, so fixed that.
  2. We have quite a few functions which (sometimes incorrectly) performed the same task of fetching an asset's current value after depreciation, so refactored all of them and now there's just one function.
  3. The fixed asset register and the chart on asset documents weren't reflecting manual depr entries, so fixed them.
  4. Wrote a patch to update value of all assets which have manual depr entries created for them.
  5. In the fixed asset report register, assets with finance books weren't filtered properly resulting in an TypeError: unsupported operand type(s) for +=: 'int' and 'NoneType' error, so fixed it.

@anandbaburajan anandbaburajan marked this pull request as draft January 23, 2023 07:45
@anandbaburajan anandbaburajan changed the title fix: get value_after_depreciation functions and manual depr entry not updating asset value fix: manual depr entry not updating asset value Jan 26, 2023
@anandbaburajan anandbaburajan marked this pull request as ready for review January 27, 2023 09:57
@anandbaburajan anandbaburajan marked this pull request as draft January 29, 2023 12:27
@anandbaburajan anandbaburajan marked this pull request as ready for review January 30, 2023 11:09
@anandbaburajan anandbaburajan changed the title fix: manual depr entry not updating asset value fix: manual depr entry not updating asset value [v14] Jan 30, 2023
@anandbaburajan anandbaburajan mentioned this pull request Jan 30, 2023
Merged
@barredterra
Copy link
Collaborator

Is there a corresponding PR that implements these changes on the develop branch?

@anandbaburajan
Copy link
Contributor Author

@barredterra not yet, I've to think more about some things. I'll open a PR in a day or two.

@deepeshgarg007 deepeshgarg007 merged commit f487eae into frappe:version-14-hotfix Jan 31, 2023
frappe-pr-bot pushed a commit that referenced this pull request Jan 31, 2023
@frappe-bot
chore(release): Bumped to Version 14.15.0
c8226ff 
# [14.15.0](v14.14.0...v14.15.0) (2023-01-31)

### Bug Fixes

* Amount validation in Payment Request against Purchase Order ([#33855](#33855)) ([5605f1e](5605f1e))
* Currency symbol for tax withholding net total field ([#33850](#33850)) ([f54e862](f54e862))
* disfuctional cost center filter on Journal Entries ([#33815](#33815)) ([58c3e16](58c3e16))
* disposal_was_made_on_original_schedule_date ([4586806](4586806))
* double salutation on quotation print ([#33834](#33834)) ([0fcf364](0fcf364))
* Fetch commission rate from sales partner ([#33851](#33851)) ([868c8d6](868c8d6))
* **gp:** fetch buying amount from dn related to so ([f5bde9c](f5bde9c))
* GST Category validation broken for pos unregistered customer who dont have address. ([#33800](#33800)) ([f124dd3](f124dd3))
* Ignore linked JE on JE cancellation ([#33852](#33852)) ([a0e1ee0](a0e1ee0))
* item rate not fetching ([b98d351](b98d351))
* Lead to customer creation ([#33859](#33859)) ([44692e9](44692e9))
* manual depr entry not updating asset value [v14] ([#33788](#33788)) ([f487eae](f487eae))
* **patch:** validation error on cost center allocation migration ([#33835](#33835)) ([5d4967c](5d4967c))
* use correct filter name in `item_query` (backport [#33814](#33814)) ([#33816](#33816)) ([f7eabca](f7eabca))

### Features

* **gp:** test for inv and dn related via so ([7a793ea](7a793ea))

### Performance Improvements

* show update items dialog ([ac2ebfb](ac2ebfb))
@frappe-pr-bot
Copy link
Collaborator

🎉 This PR is included in version 14.15.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@barredterra
Copy link
Collaborator

@anandbaburajan isn't the correct procedure to send PRs to develop, then (partially) backport them? This way we can avoid features going missing with an upgrade. For example, if somebody upgrades to develop right now, your features will not be there.

@anandbaburajan
Copy link
Contributor Author

@anandbaburajan isn't the correct procedure to send PRs to develop, then (partially) backport them?

Yep, but like I said, I've to think more about some of those features/fixes (there might be some design changes) which would go into develop and fixing bugs in v13 and v14 was the first priority so opened this PR.

@barredterra
Copy link
Collaborator

Understood. IMHO it doesn't make sense to solve a bug in a lower version and "maybe sometime" (to exaggerate) in the develop branch. This has lead to problems in the past and will continue to do so, unless we follow the proper process.

@barredterra barredterra mentioned this pull request Feb 6, 2023
Closed
@barredterra barredterra mentioned this pull request Mar 7, 2023
Closed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@deepeshgarg007 deepeshgarg007 deepeshgarg007 requested changes

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

@ruthra-kumar ruthra-kumar Awaiting requested review from ruthra-kumar ruthra-kumar is a code owner

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@anandbaburajan @barredterra @frappe-pr-bot @deepeshgarg007