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

[ADD] contract_digitized_signature #102

Merged
merged 4 commits into from
Oct 19, 2017

Conversation

cubells
Copy link
Sponsor Member

@cubells cubells commented Oct 9, 2017

New module that allows to add digitized signature to contracts and send by mail.

cc @Tecnativa

Related: #79

@cubells cubells mentioned this pull request Oct 9, 2017
2 tasks
@lasley
Copy link
Contributor

lasley commented Oct 9, 2017

Hmmmm I wonder if we need to think about integration of this against #88, which also has signature.

I'm curious though, what is the purpose of the signature field in the backend view? Is this instead for an employee to sign off on a contract completion?

@cubells
Copy link
Sponsor Member Author

cubells commented Oct 9, 2017

This module completes these others:
https://github.com/OCA/project/tree/9.0/project_task_digitized_signature
https://github.com/OCA/stock-logistics-workflow/tree/9.0/stock_picking_digitized_signature
OCA/sale-workflow#504

The purpose is that commercial user with a tablet or similar, in customer home, shows him a document (project, sale, contract or picking) and he signs document. Company can now continue commercial flow.

@cubells
Copy link
Sponsor Member Author

cubells commented Oct 9, 2017

Customer signature is shown in printed document too.

@pedrobaeza
Copy link
Member

@lasley not everyone wants to install website part (in v11 it's not needed, but it does on v10).

@rafaelbn you can try now

@pedrobaeza
Copy link
Member

@rafaelbn give your last try.

@rafaelbn rafaelbn added this to the 9.0 milestone Oct 12, 2017
@rafaelbn
Copy link
Member

Runbot says 502 Bad Gateway I cannot test it.

By the way @cubells @pedrobaeza codecov/patch — 85.71% of diff hit (target 100%)

@woodbrettm
Copy link

woodbrettm commented Oct 12, 2017

@lasley I could probs just add this module as a dependency for website_sale_contract. Most of the backend logic in website_sale_contract lies in the checkout wizard, so should not be an issue.

@cubells would it be possible to add a signature_name field to account.analytic.account? I think it would be helpful to know the name of the person that signed the contract. Would also follow what is done in website_quote.

Copy link
Member

@chienandalu chienandalu left a comment

Choose a reason for hiding this comment

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

Tested on runbot. Just two non blocking comments


This module is part of the OCA/web suite.

Configuration
Copy link
Member

Choose a reason for hiding this comment

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

Is this section needed. It's the same instructions as in Usage

changes on it.
#. You can clear signature in any moment by clicking on the **Clear** link
located on the top right side of signature box.
#. If you print Contract, signature was added to contact report .
Copy link
Member

Choose a reason for hiding this comment

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

If you print the contract the signature will be shown in the contract report.

@lasley
Copy link
Contributor

lasley commented Oct 13, 2017

@BMW95 - if you can easily make our module compatible with this strategy, please do it. I'd rather not have a fractured ecosystem

@rafaelbn
Copy link
Member

Please @cubells could you answer to #102 (comment). Thanks!

Copy link
Member

@rafaelbn rafaelbn left a comment

Choose a reason for hiding this comment

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

Tested functionaly in runbot 👍

@cubells
Copy link
Sponsor Member Author

cubells commented Oct 16, 2017

@BMW95 @lasley
I added a new field signature_name

Can you retest?

@@ -12,6 +12,11 @@ class AccountAnalyticAccount(models.Model):
customer_signature = fields.Binary(
string='Customer acceptance',
)
signature_name = fields.Many2one(
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO this should be a plain text field, possibly containing a capture of the name from the partner (not sure how your workflow works). Linking a signature to a dynamic record means that the actual data can change out from underneath the legal data

Copy link
Member

Choose a reason for hiding this comment

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

Well, you shouldn't modify contacts when there are changes, but deactivate them and create new ones. The problem with the plain text is that requires to fully type it. With this, you can select directly by direct search.

Copy link
Contributor

Choose a reason for hiding this comment

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

@pedrobaeza - none of my customers do that, so while yeah it's the best option, it's still not a legal field.

An alternative would be to add a signatory_id column that is the relation, with an onchange to update the signature_name.

Furthermore- this creates an incompatibility with the way Odoo does it, which is the way we did it. The goal of adding this was to make our addons live in harmony. Adding this, but making it a relation, takes us further from that goal than simply not having the column

Copy link
Contributor

@lasley lasley Oct 17, 2017

Choose a reason for hiding this comment

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

Also - to be a relation, it needs to be suffixed with _id, which is not the case with signature_name

Copy link
Member

Choose a reason for hiding this comment

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

OK, let's change it to a char field. At the end, we don't have that requirement.

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

done!

Copy link
Contributor

@lasley lasley left a comment

Choose a reason for hiding this comment

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

Thanks @cubells. One note, non-functional change

def create(self, values):
contract = super(AccountAnalyticAccount, self).create(values)
if contract.customer_signature:
values = {'customer_signature': contract.customer_signature}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a test for this please?

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

@lasley done!

@lasley lasley merged commit 59f07d1 into OCA:9.0 Oct 19, 2017
@lasley
Copy link
Contributor

lasley commented Oct 19, 2017

Thanks @cubells

@pedrobaeza pedrobaeza deleted the 9.0-mig-contract_digitized_signature-2 branch October 19, 2017 14:50
woodbrettm pushed a commit to LasLabs/contract that referenced this pull request Oct 25, 2017
* [ADD] contract_digitized_signature

* Correct README and add signature_name

* Change to char field

* Improve tests to acchieve full coverage
woodbrettm pushed a commit to LasLabs/contract that referenced this pull request Nov 15, 2017
* [ADD] contract_digitized_signature

* Correct README and add signature_name

* Change to char field

* Improve tests to acchieve full coverage
woodbrettm pushed a commit to LasLabs/contract that referenced this pull request Dec 2, 2017
* [ADD] contract_digitized_signature

* Correct README and add signature_name

* Change to char field

* Improve tests to acchieve full coverage
cubells added a commit to Tecnativa/contract that referenced this pull request Apr 19, 2019
* [ADD] contract_digitized_signature

* Correct README and add signature_name

* Change to char field

* Improve tests to acchieve full coverage
cubells added a commit to Tecnativa/contract that referenced this pull request Apr 19, 2019
* [ADD] contract_digitized_signature

* Correct README and add signature_name

* Change to char field

* Improve tests to acchieve full coverage
cubells added a commit to cubells/contract that referenced this pull request Aug 16, 2019
* [ADD] contract_digitized_signature

* Correct README and add signature_name

* Change to char field

* Improve tests to acchieve full coverage
carms-ng pushed a commit to ERPLibre/contract that referenced this pull request Oct 14, 2020
* [ADD] contract_digitized_signature

* Correct README and add signature_name

* Change to char field

* Improve tests to acchieve full coverage
mathben pushed a commit to TechnoLibre/contract that referenced this pull request Mar 8, 2021
* [ADD] contract_digitized_signature

* Correct README and add signature_name

* Change to char field

* Improve tests to acchieve full coverage
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants