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

Fixed asset refactor #19369

Merged
merged 50 commits into from
Nov 18, 2019
Merged

Conversation

nextchamp-saqib
Copy link
Member

@nextchamp-saqib nextchamp-saqib commented Oct 21, 2019

  • Item Master & Asset
    • Serial No, Serial Number Series is now disabled for Fixed Asset Item
    • Auto Create Asset on Purchase Checkbox for Fixed Asset Item
    • Mandatory Asset Category & Naming Series for auto creation
    • Manual Asset creation should be linked with a purchase document
  • Asset Movement
    • Has table for list of assets
    • List has Make Asset Movement Action for a selected no. of assets
    • Employee is linked to Asset Custodian field
  • Purchase Receipt
    • Asset are created based on qty when auto create checked in item. Initial Asset Movement is created
    • Cancelling of PR requires deletion of Asset Movement and cancelling of Linked Asset
  • Purchase Invoice
    • Asset are created based on qty when auto create checked in item & update stock is checked on PI. Initial Asset Movement is created
    • Cancelling of PI is same as Purchase Receipt
  • Landed Cost Voucher for Asset
    • Validates assets created is same as purchase receipt/invoice item quantity
    • Removed serialised assets rate updation
    • Asset gross amount are updated when GL Entries are created

@@ -302,6 +333,65 @@ frappe.ui.form.on('Asset', {
})
},

purchase_receipt: function(frm) {
Copy link
Member

Choose a reason for hiding this comment

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

Write set_query for purchase receipt and invoice link fields. it should only show the receipts/invoices where item used.


if asset_movement:
doc = frappe.get_doc('Asset Movement', asset_movement)
# should this be hard coded ?
Copy link
Member

Choose a reason for hiding this comment

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

remove naming series from asset movement

@@ -439,11 +453,12 @@ def make_purchase_invoice(asset, item_code, gross_purchase_amount, company, post
pi.currency = frappe.get_cached_value('Company', company, "default_currency")
pi.set_posting_time = 1
pi.posting_date = posting_date

Copy link
Member

Choose a reason for hiding this comment

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

remove entire make_purchase_invoice functionality

if latest_movement_entry:
location = latest_movement_entry[0][0]
employee = latest_movement_entry[0][1]
elif self.purpose in ['Transfer', 'Receipt']:
Copy link
Member

Choose a reason for hiding this comment

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

Refactor this function. On cancellation, ensure atleast one movement entry always exists

elif asset.status in ("Scrapped", "Cancelled", "Sold"):
frappe.throw(_("Row #{0}: Asset {1} cannot be submitted, it is already {2}")
.format(d.idx, d.asset, asset.status))
if d.is_fixed_asset and d.meta.get_field("asset") and d.asset:
Copy link
Member

Choose a reason for hiding this comment

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

move function to sales_invoice.py


cwip_disabled = is_cwip_accounting_disabled()
return gl_entries
Copy link
Member

Choose a reason for hiding this comment

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

no need to return gl_entries

if asset_rbnb_currency == self.company_currency else asset_amount)
}, item=item))

return gl_entries
Copy link
Member

Choose a reason for hiding this comment

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

no need to return gl_entries


if not expenses_included_in_valuation:
expenses_included_in_valuation = self.get_company_default("expenses_included_in_valuation")
return gl_entries
Copy link
Member

Choose a reason for hiding this comment

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

no need to return gl_entries


return gl_entries
for asset in assets:
doc = frappe.get_doc("Asset", asset.name)
Copy link
Member

Choose a reason for hiding this comment

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

doc is not used

}, account_currency, item=item))

# If asset is bought through this document and not linked to PR
if self.update_stock:
Copy link
Member

Choose a reason for hiding this comment

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

and item.landed_cost_voucher_amount

* make asset movement on asset submission
* add PR & PI queries based on item code
@nabinhait nabinhait added this to the Release - November 2019 milestone Nov 12, 2019
@nabinhait nabinhait self-assigned this Nov 13, 2019
@@ -380,7 +382,7 @@ def add_asset_gl_entries(self, item, gl_entries):

def add_lcv_gl_entries(self, item, gl_entries):
expenses_included_in_asset_valuation = self.get_company_default("expenses_included_in_asset_valuation")
if is_cwip_accounting_disabled():
if is_cwip_accounting_enabled(self.company, item.asset_category):
Copy link
Member

Choose a reason for hiding this comment

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

if not is_cwip_accounting_enabled?

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

if not is_cwip_accounting_disabled():
if not item.asset_category:
item.asset_category = frappe.get_cached_value("Item", item.item_code, "asset_category")
if not is_cwip_accounting_enabled(self.company, item.asset_category):
Copy link
Member

Choose a reason for hiding this comment

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

if is_cwip_accounting_enabled?

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

@nabinhait nabinhait merged commit d995609 into frappe:develop Nov 18, 2019
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.

3 participants