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] add crm_lead_lost_reason addon #16

Merged
merged 11 commits into from Feb 25, 2015

Conversation

rdeheele
Copy link

Add opportunity_lost_reason addon (sale_cancel_reason clone)

'images': [],
'website': "http://www.camptocamp.com",
'description': """
Opportunity Lost Reason
Copy link
Member

Choose a reason for hiding this comment

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

Extract to README.rst.

@pedrobaeza
Copy link
Member

Please call the module crm_lead_lost_reason, to be technically coherent, although you put on the description the word "Opportunity".

@dreispt
Copy link
Sponsor Member

dreispt commented Feb 17, 2015

Isn't this already implemented on sale_cancel_reason?

@pedrobaeza
Copy link
Member

As I can see, it's not the same part of the sales flow: one is in the opportunity, the other on the sale order.

@dreispt
Copy link
Sponsor Member

dreispt commented Feb 18, 2015

OK

@rdeheele
Copy link
Author

@pedrobaeza agree with you to rename the module.
it's work in progress because I see now 2 ways to avoid providing a lost reason:

  • via kanban view
  • via tree view and the button "More".

@coveralls
Copy link

Coverage Status

Coverage remained the same at 95.65% when pulling 2285e8b on rdeheele:opportunity_lost_reason into 67970e8 on OCA:8.0.

@rdeheele
Copy link
Author

@pedrobaeza @dreispt @yvaucher feedbacks are welcome.
In the last commit, on kanban/list views, we force the users to pass by form to mark as lost an opportunity (a warning is raised to inform about it) : it avoids struggling with javascript kanban view code.

@pedrobaeza
Copy link
Member

Well, check the stage by name is not the best option, and it can be conflicted also with languages. There isn't any other option?

@rdeheele
Copy link
Author

gaah..there are 4 ways to mark lost an opportunity (via more button on list view, via drag and drop on kanban view, via click on statusbar on form view and ... via the button "Mark Lost" on form)

@coveralls
Copy link

Coverage Status

Coverage remained the same at 93.75% when pulling 5d88be3 on rdeheele:opportunity_lost_reason into 67970e8 on OCA:8.0.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 93.75% when pulling 0ffa89c on rdeheele:opportunity_lost_reason into 67970e8 on OCA:8.0.

@rdeheele
Copy link
Author

there are multiple ways to set "Lost" a lead (more button on list view, drag and drop on kanban view,click on statusbar on form view and the red button "Mark Lost" on form), for now this addon forces the user to use the red button "Mark Lost" on form : this is the only way where it's easy (i.e. without dealing with javascript) to display a pop-up wizard for the user to indicate a lost reason.

@pedrobaeza
Copy link
Member

Well, it's not perfect, but it can work. Please put this on README file under section Known issues / Roadmap. Also OCA's template would be advisable.

@jgrandguillaume
Copy link
Member

Hi,

Thanks for the contrib ! +1 to have the OCA template. Otherwise LGTM 👍

Regards,

@@ -0,0 +1,23 @@
Opportunity Lost Reason
Copy link
Member

Choose a reason for hiding this comment

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

this needs to be a .rst file. Just renaming should be ok

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

@gurneyalex afaik markdown is allowed.

Copy link
Member

Choose a reason for hiding this comment

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

No, not really, they are read, but not correctly parsed. They are parsed as RestructuredText (RST). So better to put .rst so GitHub parses OK also.

@gurneyalex
Copy link
Member

missing .pot file

@rdeheele rdeheele changed the title [ADD] add opportunity_lost_reason addon [ADD] add crm_lead_lost_reason addon Feb 25, 2015
@coveralls
Copy link

Coverage Status

Coverage remained the same at 93.75% when pulling c89ed8d on rdeheele:opportunity_lost_reason into 67970e8 on OCA:8.0.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 93.75% when pulling c89ed8d on rdeheele:opportunity_lost_reason into 67970e8 on OCA:8.0.

@pedrobaeza
Copy link
Member

Thanks 👍 if Travis goes well.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 93.75% when pulling c89ed8d on rdeheele:opportunity_lost_reason into 67970e8 on OCA:8.0.

lepistone added a commit that referenced this pull request Feb 25, 2015
[ADD] add crm_lead_lost_reason addon
@lepistone lepistone merged commit 90284e1 into OCA:8.0 Feb 25, 2015
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.

None yet

8 participants