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

[10.0][ADD] website_sale_contract #88

Conversation

woodbrettm
Copy link

@woodbrettm woodbrettm commented Aug 10, 2017

TODO

  • JS Tests
  • Backend Tests
  • Contract step doesn't turn Green after done
  • Add security
  • Ensure users can't go to unsigned contracts in UI using url.
  • Ensure users can't go to contracts in other checkout wizards using url.
  • Ensure fields highlighted in JS Signing Form when errors
  • Ensure proper fields are populated in account.analytic.account when converting temp contracts to account.analytic.account.
  • Ensure contracts are converted to account.analytic.accounts at correct stage (currently done during confirmation stage)
  • Fix pricing section in contract page.
  • The Confirmation Message in the contract signing modal is incomplete. (i.e. "I accept the terms of . . .", then does signature and types name.)
  • Adjust top margin in mobile for contract header name

@woodbrettm woodbrettm force-pushed the feature/10.0/LABS-449-create-website_sale_contract branch from 97e3c15 to 08a9265 Compare August 10, 2017 17:28

ContractCheckout = self.env['contract.checkout']

contract_wizard = ContractCheckout.search(['order_id', '=', order.id])
Copy link
Contributor

Choose a reason for hiding this comment

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

You should instead give your wizard model a method that takes an order ID and returns existing or creates new

})

if not contract_wizard.contract_ids:
contract_wizard._create_temp_lines(sale_lines)
Copy link
Contributor

Choose a reason for hiding this comment

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

What if the sale lines change after going back? I think instead your model will need to have a method to manage differentials

return super(WebsiteSale, self).payment_confirmation(**post)


class SaleContract(http.Controller):
Copy link
Contributor

Choose a reason for hiding this comment

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

Bad indentation

sale_order_id = request.session.get('sale_last_order_id')
# if not accounts created,
# do _convert_temp_lines_to_accounts() in contract_checkout
# how to tell if been created?
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand this comment/question. Elaborate?

lambda s: s.product_id.is_contract
)

sale_lines = sorted(lambda s: s.id, sale_lines)
Copy link
Contributor

Choose a reason for hiding this comment

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

Move this sort below the if not sale_lines. Why sort if non-existent?

License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). -->

<odoo>

Copy link
Contributor

Choose a reason for hiding this comment

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

Blank?

@api.multi
def _create_temp_lines(self, sale_order_lines):
self.ensure_one()
ContractTemp = self.env['contract.temp']
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems pretty inefficient & I'm not fully understanding what you're going for. Is this so that you can create a linked list type hierarchy of the contracts for iteration? Is there any other point to the temp contracts?

Copy link
Author

@woodbrettm woodbrettm Aug 16, 2017

Choose a reason for hiding this comment

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

Yeah exactly about the linked list. That's so they can be looped through during checkout.

You mentioned in the #65 that the contracts won't be created until the final payment process has been completed, so in order to ensure those contracts have the signatory field data saved, there needed to be somewhere to temporarily save them.

for record in self:
for contract in record.contract_ids:
vals = {

Copy link
Contributor

Choose a reason for hiding this comment

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

Blank

string='Contract Wizard',
comodel_name='contract.checkout',
)
contract_signature = fields.Binary(
Copy link
Contributor

Choose a reason for hiding this comment

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

signature_image

contract_signature = fields.Binary(
string='Contract Signature',
)
signatory = fields.Char(
Copy link
Contributor

Choose a reason for hiding this comment

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

signature_name

@lasley
Copy link
Contributor

lasley commented Aug 15, 2017

@yelizariev - FYI we're going to use this to upgrade the e-commerce purchase workflow provided by your SaaS tools, so it may interest you to look at this.

@woodbrettm
Copy link
Author

woodbrettm commented Aug 16, 2017

@lasley here is a quick mockup of the contract view in checkout. Thought this might be an interesting thing to try earlier on since it can be so expensive changing things after the fact in dev.

I'm considering moving that 'Accept Order` modal to the left side instead, to match what's done in website_quote (just to ensure js will work when inheriting)

Perhaps moving the Back and Next buttons to the bottom might be a better as well.

website_sale_contract

@lasley
Copy link
Contributor

lasley commented Aug 16, 2017

Thanks for the mockup @BMW95 - I sent out to a few stakeholders for review & will ponder over the weekend

@lasley
Copy link
Contributor

lasley commented Aug 29, 2017

@BMW95 feedback on the mockup:

  • I think we should move the headers to the left side
  • back/next should replace Accept Order in the headers div
  • Agreed on pricing header
  • We probably don't want the history (messaging) until after the contract is signed. I'm not sure how that would work in terms of responses and stuff

Everything else seems fine - basically identical to the portal, which is what we're going for.

@rafaelbn
Copy link
Member

Hi @BMW95 , could you please rebase? once #62 is merged we should be able to test this one! Thanks! 😄

@rafaelbn rafaelbn added this to the 10.0 milestone Oct 12, 2017
@woodbrettm
Copy link
Author

@rafaelbn I have some local code that's not quite ready to be pushed yet, but once it is I'll rebase :)

@woodbrettm woodbrettm force-pushed the feature/10.0/LABS-449-create-website_sale_contract branch from 08a9265 to c177d3b Compare October 25, 2017 20:54
@woodbrettm woodbrettm force-pushed the feature/10.0/LABS-449-create-website_sale_contract branch 2 times, most recently from 4d3344f to a376f83 Compare November 18, 2017 01:10
@woodbrettm
Copy link
Author

@lasley

Alright so this is ready for first round of review. I added a TODO at the top of the PR for the remaining things that need to be done so will be working on those.

Page of a contract provided below. The Sign Contract button disappears and a Next link shows up next to Back when the contract has been signed.

Couple Initial Questions

  • What should go in the Pricing Section? Perhaps we don't even need one and can just have the title of the contract and then the terms.
  • The temp contracts are being turned into real ones at the confirmation step. I noticed also at this step a quote for the order is created. I'm not sure having the contracts created before a quote for the order has been accepted, so just bringing that up for discussion as to how it should be properly done.

fireshot capture 16 - contract checkout i website localhost_ - http___192 168 1 226_8069_shop_che

@lasley
Copy link
Contributor

lasley commented Nov 20, 2017

Thanks for the update @BMW95 - I'll review the code soon.

To answer your questions:

The pricing section should be switched to say "Terms" I think. Those terms would include how often the invoices for this contract are being created, and the invoice lines themselves would be in the table that's currently there.

Isn't an actual sale order created during the confirmation step? AFAIK it's a quote throughout everything, but once the payment happens it should be an order in the sale state.

@lasley
Copy link
Contributor

lasley commented Nov 20, 2017

Oh and we should probably include the product that was linked to the contract in the terms as well. It would be akin to a setup fee.

@pedrobaeza
Copy link
Member

Is this going to be finished?

@woodbrettm
Copy link
Author

@pedrobaeza yup! Sorry just tied up with another item right now. Will be back on this soon.

@woodbrettm woodbrettm force-pushed the feature/10.0/LABS-449-create-website_sale_contract branch from a376f83 to 3af6b0a Compare January 11, 2018 01:02
@woodbrettm woodbrettm force-pushed the feature/10.0/LABS-449-create-website_sale_contract branch from 3af6b0a to b332dfb Compare January 11, 2018 01:36
@woodbrettm woodbrettm force-pushed the feature/10.0/LABS-449-create-website_sale_contract branch from b332dfb to c3d2c84 Compare January 11, 2018 01:50
@woodbrettm
Copy link
Author

Aight this is ready for first round of review. Travis is failing from something in contract.

Pic below of contract view.

contract

@woodbrettm
Copy link
Author

Also an odd thing that happens I've noticed ever since working on this module is the following error when recurring_create_invoice() is called in product_contract (in action_confirm).

  File "/opt/odoo/odoo/http.py", line 638, in _handle_exception
    return super(JsonRequest, self)._handle_exception(exception)
  File "/opt/odoo/odoo/http.py", line 675, in dispatch
    result = self._call_function(**self.params)
  File "/opt/odoo/odoo/http.py", line 331, in _call_function
    return checked_call(self.db, *args, **kwargs)
  File "/opt/odoo/odoo/service/model.py", line 119, in wrapper
    return f(dbname, *args, **kwargs)
  File "/opt/odoo/odoo/http.py", line 324, in checked_call
    result = self.endpoint(*a, **kw)
  File "/opt/odoo/odoo/http.py", line 933, in __call__
    return self.method(*args, **kw)
  File "/opt/odoo/odoo/http.py", line 504, in response_wrap
    response = f(*args, **kw)
  File "/opt/odoo/addons/website_quote/controllers/main.py", line 89, in accept
    Order.action_confirm()
  File "/opt/odoo/addons/website_quote/models/sale_order.py", line 176, in action_confirm
    res = super(SaleOrder, self).action_confirm()
  File "/media/sf_odoo10dev/contract/product_contract/models/sale_order.py", line 24, in action_confirm
    contract.recurring_create_invoice()
  File "/media/sf_odoo10dev/contract/contract/models/account_analytic_account.py", line 288, in recurring_create_invoice
    invoices |= contract.with_context(ctx)._create_invoice()
  File "/media/sf_odoo10dev/contract/contract/models/account_analytic_account.py", line 252, in _create_invoice
    invoice_vals = self._prepare_invoice()
  File "/media/sf_odoo10dev/contract/contract/models/account_analytic_account.py", line 247, in _prepare_invoice
    return invoice._convert_to_write(invoice._cache)
  File "/opt/odoo/odoo/models.py", line 4898, in _convert_to_write
    value = field.convert_to_cache(value, self, validate=False)
  File "/opt/odoo/odoo/fields.py", line 1884, in convert_to_cache
    return process((value[0],))
IndexError: tuple index out of range

@lasley
Copy link
Contributor

lasley commented Jan 12, 2018

Also an odd thing that happens I've noticed ever since working on this module is the following error when recurring_create_invoice() is called in product_contract (in action_confirm).

Does the error exist without this module installed?

Travis is failing from something in contract.

Unrelated. It looks to be failing on the main branch for the same reason.

@pedrobaeza
Copy link
Member

Travis error is not unrelated. Main branch is green (there were some Travis glitches only).

@woodbrettm
Copy link
Author

woodbrettm commented Jan 12, 2018

I double checked the test_automatic_price test fail locally both on 10.0 and this branch, and it passes here, interestingly. However many of the tests show the tuple index out of range error. Outside of tests, to recreate the error I did the following:

  • Fresh db on 10.0
  • Install Accounting and Finance
  • Install Contract
  • Enable developer mode
  • Go to contracts, open view to create a contract.
  • Specify customer as Agrolait, account name is Test. Add a single Graphics Card invoice line.
  • Save. Then I click 'Create Invoices'. The tuple error shows.

@github-actions
Copy link

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Oct 24, 2021
@github-actions github-actions bot closed this Nov 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs review stale PR/Issue without recent activity, it'll be soon closed automatically.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants