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

New features to added PDC(Post Dated Cheque) Management. #12961

Closed
wants to merge 9 commits into from
Closed

New features to added PDC(Post Dated Cheque) Management. #12961

wants to merge 9 commits into from

Conversation

Solufyin
Copy link
Contributor

@revant, @manassolanki
We added a new Feature to PDC(Post Dated Cheque) Management for BANK RECONCILIATION functionality.

customer

bank recon-statement1

bank recon-statement2

You can refer attached video for the PDC Management:

PDC Workflow for ERPNEXT.mp4.zip

Copy link
Member

@rmehta rmehta left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

Please see my comment on naming and do include relevant test cases and documentation.

"width": 110
},
{
"fieldname": "cheque_status",
Copy link
Member

Choose a reason for hiding this comment

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

can we use deposit status?

The word "cheque" has a spelling problem (its check in en-US)

.format(d.idx, d.return_date, d.cheque_date))

if d.return_date or self.include_reconciled_entries:
if not d.return_date:
Copy link
Member

Choose a reason for hiding this comment

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

what is the use of this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are updating the status of payment entry by updating return date to related payment entry.

if d.return_date:
payment_ref = frappe.get_doc(d.payment_document, d.payment_entry)

cheque_status = "Return"
Copy link
Member

Choose a reason for hiding this comment

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

deposit_status

@rmehta rmehta added needs-tests This PR needs automated unit-tests. needs-docs labels Feb 22, 2018
@Solufyin
Copy link
Contributor Author

Dear @rmehta and @revant ,

We improved code as per your suggestion and here attached documentation regarding "PDC Management workflow" with snapshot.

PDC Management Workflow1.pdf

@nabinhait
Copy link
Member

@Solufyin Thanks for the pull request. But there is another pull request to manage payment return sent by @tundebabzy #11896 . And we think the approach implemented by @tundebabzy is better than this, mainly because "Explicit is better than implicit" and it gives more flexibility. He basically created another Journal Entry for tracking the return.
Hence, I am closing this pull request.
@tundebabzy Can you please resend the PR after adding tests?

@nabinhait nabinhait closed this Mar 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-docs needs-tests This PR needs automated unit-tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants