Closed agritheory closed 6 years ago
Hi @agritheory What's the status of this issue?
Hey @tundebabzy I've been stuck on other things since I posted it. I'd like to take a pass at it this weekend. I'll tag you with any updates, because while I don't think I'll be able to solve all of it myself, I'd like to try. Also, I love your role as the official fire chief of ERPNext. Keep up the excellent work. If appropriate, please assign other users to help.
https://github.com/frappe/erpnext/pull/11274
@tundebabzy please review
@agritheory I made some comments on the PR
@agritheory do you still need any assistance
Hi @tundebabzy , I think I've got it, had to update to V9. But yes, I think I need some git help. I have edited the following files and these are all the ones I want to put into the pull request. I have not updated the tests and would like your opinion if it is needed. What is the best way to show you the changes? asset.js asset.json asset.py asset_category.js asset_category.json asset_category.py
I don't know any clean way other than starting over.
I did start over, I made the made the changes and I think they're right. I will do a pull request and only include those files. I don't have a lot of experience with that and I suspect it won't meet the guidelines, but I'm appreciative of the feedback.
As long as you started by creating your branch from the latest develop branch, your pr should be fine. Do take note that breaking the PR into small commits would make review easier.
You can uncheck "Calculate Depreciation"
Some assets cannot be depreciated (at least according to IRS rules), most notably land but sometimes others as well (orchard crops in their pre-productive phase, sometimes goodwill). It's also a fine thing for putting an asset in an "ask my accountant" state.
I propose to add this category to assets with the deprecation method select:
When 'Non-Depreciable Asset' is selected, the depreciation field should change from required to hidden. The user should also be warned or asked to confirm about selecting this since it's mostly a special case.