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

[MERGE][ADD] website_slides_sale: allow to sell slide.channel #30914

Closed
wants to merge 4 commits into from

Conversation

awa-odoo
Copy link
Contributor

@awa-odoo awa-odoo commented Feb 7, 2019

Purpose of this merge is to be able to sell courses [1] when using the
slides / eLearning platform.

This commit adds sale capabilities on a slide.channel. A slide.channel can
now have the 'payment' visibility, that requires a 'product_id' configured
on the channel.

When a customer purchases a product linked to a channel, he is added to the
members of the channel (see slide.channel.partner_ids) when his order is
confirmed.

This merge is linked to task ID 1937160 and PR #30914. Future tasks will
improve homepage of channels and clearly show public, private and payment
based channels [2].

[1] see task ID 1902304 (main eLearning task) PR #29876;
[2] see task ID 1936153 (new homepage for slides) PR #30770;

@C3POdoo C3POdoo added the RD research & development, internal work label Feb 7, 2019
@robodoo robodoo added the CI 🤖 Robodoo has seen passing statuses label Feb 7, 2019
robodoo added a commit that referenced this pull request Feb 7, 2019
…profile

Purpose of this merge is to prepare addition of gamification in slides /
eLearning platform. In order to be able to use karma and the badges in other
modules we move those models in gamification.

A website_profile module is added to hold code, data and all necessary stuff
to display a user profile, including its gamification-related details. It
is a bridge between gamification and website. It will soon hold a revamped
user profile, used in both forum and slides.

More generally this merge is linked to ongoing tasks
 * task ID 1902304 (main eLearning task) PR #29876;
 * task ID 1922159 (new user profile and gamification) PR #30514;
 * task ID 1936153 (new homepage for slides) PR #30770;
 * task ID 1937160 (payment flow and integration with ecommerce) PR #30914;

This merge is therefore not functionally complete itself and serves as a
preparation for other incoming commits. See sub commits for more details
on what has been done here.

closes #30908
robodoo added a commit that referenced this pull request Feb 7, 2019
Purpose of this merge is to clean access rights on slide models and give
less access to public / portal / internal users. Website publishers and
editors have rights to edit slides content and manage channels.

Another purpose is to clean and refactor access options on channels and slides.
Access to a given slide depends on the current user being a member of the
slide channel. A channel can be either public (joined easily) or on invitation
only (manual addition of members).

Slide can be flagged as preview, meaning they are accessible without any
concern about channel membership. It is used as teasing or free promotion
of a channel.

Publish flag is orthogonal. Non published slides are not visible to everyone
except website content editor. It allows to work and improve slides without
displaying them to members or public people.

Slide upload access is a bit cleaned. A can_publish field is added controlling
who can publish slides. Currently website publishers are allowed to publish
content. Not publishers people allowed to upload cannot publish their own
content. It stays in a submitted non published state.

Base for tests are also added. More of them will be added in future commits.

This merge is related to task ID 1937411 and closes PR #30847. More
generally this merge is linked to ongoing tasks

* task ID 1902304 (main eLearning task) PR #29876;
* task ID 1922159 (new user profile and gamification) PR #30514;
* task ID 1936153 (new homepage for slides) PR #30770;
* task ID 1937160 (payment flow and integration with ecommerce) PR #30914;

This merge is therefore not functionally complete itself and serves as a
preparation for other incoming commits. See sub commits for more details
on what has been done here.
@awa-odoo awa-odoo force-pushed the master-slides-sell-courses-awa branch from e54b688 to a3f8481 Compare February 8, 2019 10:48
@robodoo robodoo removed the CI 🤖 Robodoo has seen passing statuses label Feb 8, 2019
@awa-odoo awa-odoo force-pushed the master-slides-sell-courses-awa branch from a3f8481 to f36f979 Compare February 8, 2019 10:51
@awa-odoo awa-odoo changed the title Master slides sell courses awa [ADD] website_slides_sale: allow to sell slide.channel Feb 8, 2019
@awa-odoo awa-odoo force-pushed the master-slides-sell-courses-awa branch 3 times, most recently from ee8167d to 53628d0 Compare February 11, 2019 08:30
@robodoo robodoo added the CI 🤖 Robodoo has seen passing statuses label Feb 11, 2019
Copy link
Contributor

@tde-banana-odoo tde-banana-odoo left a comment

Choose a reason for hiding this comment

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

Technical review ! Woop woop.

subject = fields.Char('Subject')
body = fields.Html('Contents', default='', sanitize_style=True)
attachment_ids = fields.Many2many(
'ir.attachment', 'slide_channel_mail_compose_message_ir_attachments_rel', 'wizard_id', 'attachment_id',
Copy link
Contributor

Choose a reason for hiding this comment

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

Small note: I think we can either rely on orm standard table name and not give it, either give something shorter ^^

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. (also done for partner_ids)

'ir.attachment', 'slide_channel_mail_compose_message_ir_attachments_rel', 'wizard_id', 'attachment_id',
string='Attachments')
template_id = fields.Many2one(
'mail.template', 'Use template', index=True,
Copy link
Contributor

Choose a reason for hiding this comment

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

Small note: index is not necessary

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

'mail.template', 'Use template', index=True,
domain="[('model', '=', 'slide.channel.partner')]")
# origin
email_from = fields.Char('From', default=_get_default_from, help="Email address of the sender.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Small note: help does not add anything I think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

author_id = fields.Many2one(
'res.partner', 'Author', index=True,
ondelete='set null', default=_get_default_author,
help="Author of the message.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Small note: help does not add anything I think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

# origin
email_from = fields.Char('From', default=_get_default_from, help="Email address of the sender.")
author_id = fields.Many2one(
'res.partner', 'Author', index=True,
Copy link
Contributor

Choose a reason for hiding this comment

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

Small note: index is not necessary

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


@api.onchange('template_id')
def _onchange_template_id(self):
""" UPDATE ME """
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👼

@@ -143,6 +151,44 @@ def _compute_website_url(self):
def change_visibility(self):
pass

@api.multi
def action_redirect_to_members(self):
view_list_id = self.env.ref('website_slides.view_slide_channel_partner_tree').id
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we could have at least one action in data, notably if we want to add a menu entry in debug mode somewhere to have access to channel members. Then just read it and tweak domain / context if necessary to avoid having to declare an action in python.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be ok now if I understood what you meant correctly.
(Could be wise to double check though).

# -*- coding: utf-8 -*-
# Part of Odoo. See LICENSE file for full copyright and licensing details.
{
'name': "website_slides_sale",
Copy link
Contributor

Choose a reason for hiding this comment

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

As name is displayed in the apps kanban view it should have a sexy label :) . Something like "Sell Courses" probably.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops. Fixed.

as a member of the channel(s) on which this product is configured (see slide.channel.product_id). """
result = super(SaleOrder, self)._action_confirm()

for sale_order in self:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we could improve a bit computation here to try to speedup things. Indeed we have a mapped and a search inside a loop which will scale badly if recordset grows.

Maybe we could try doing a two-step search
1/ search all products related to courses
2/ search so lines with so_id in self.ids, product_id in products.ids
Then add member accordingly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did everything we talked about except the "list comprehension" since i use the "|" operator and it does not work (I think?) with a list.

Anyway it should be a lot quicker now.
I also modified the test a little to check for multiple channels configured with the same product.

Let me know if I still need to change anything.

@@ -0,0 +1,2 @@
id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink
access_website_slides_sale_website_slides_sale,website_slides_sale.website_slides_sale,model_website_slides_sale_website_slides_sale,,1,0,0,0
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this access right ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I should be more careful with 'scaffold' apparently, I removed it.

robodoo added a commit that referenced this pull request Feb 11, 2019
Purpose of this merge is to prepare eLearning feature by already modifying
channel model.

It includes

 * addition of tag and tag groups on channel, allowing to filter and search;
 * addition of statistics computation on channel, notably tracking completion
   of users;
 * removal of promoted slide feature and addition of specific image field
   on channel;

This merge is related to task ID 1936153 and closes PR #30985. More
generally this merge is linked to ongoing tasks

* task ID 1902304 (main eLearning task) PR #29876;
* task ID 1922159 (new user profile and gamification) PR #30514;
* task ID 1937160 (payment flow and integration with ecommerce) PR #30914;
Copy link
Contributor

@jem-odoo jem-odoo left a comment

Choose a reason for hiding this comment

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

Small review with small comment for a better world. Nothing transcendent.
Bisous



class ResConfigSettings(models.TransientModel):
_inherit = "res.config.settings"

website_slide_google_app_key = fields.Char(related='website_id.website_slide_google_app_key', readonly=False)
module_website_sale = fields.Boolean("Sell courses")
Copy link
Contributor

Choose a reason for hiding this comment

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

Put directly the bridge module website_sale_slides

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

_description = 'Channel Invitation Wizard'

@api.model
def _get_default_from(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

_default_email_from to be guidelines compliant

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

raise UserError(_("Unable to post message, please configure the sender's email address."))

@api.model
def _get_default_author(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

_Default_author_id to be guidelines compliant

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

<?xml version="1.0" encoding="utf-8"?>
<odoo>
<data>
<record model="ir.ui.view" id="slide_channel_invite_view_form">
Copy link
Contributor

Choose a reason for hiding this comment

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

id first, then model.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The world is now a little better thanks to you <3

@@ -104,6 +114,31 @@
</field>
</record>

<record id="view_slide_channel_partner_tree" model="ir.ui.view">
Copy link
Contributor

Choose a reason for hiding this comment

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

slide_channel_partner_view_tree

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

<?xml version="1.0" encoding="utf-8"?>
<odoo>
<data>
<record model="ir.ui.view" id="view_slide_channel_form_inherit_website_slides_sale">
Copy link
Contributor

Choose a reason for hiding this comment

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

id first.
id="slide_channel_view_form". 'inherit' is not mandatory anymore, as we are in a bridge module, the prefix will tell us this is a view extension.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@awa-odoo awa-odoo force-pushed the master-slides-sell-courses-awa branch from 86a97d5 to 29b4f85 Compare February 11, 2019 13:21
@robodoo robodoo removed the CI 🤖 Robodoo has seen passing statuses label Feb 11, 2019
@awa-odoo awa-odoo force-pushed the master-slides-sell-courses-awa branch 4 times, most recently from 77d0d60 to fbf1b0b Compare February 11, 2019 15:06
@robodoo robodoo added the CI 🤖 Robodoo has seen passing statuses label Feb 11, 2019
Copy link
Contributor

@tde-banana-odoo tde-banana-odoo left a comment

Choose a reason for hiding this comment

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

Review: LGTM, some small comments. I did not check commit messages, feel free to update them so that we can r+ soon :) . Bisous.

_inherit = 'slide.channel'

visibility = fields.Selection(selection_add=[('payment', 'On payment')])
product_id = fields.Many2one('product.product', 'Product')
Copy link
Contributor

Choose a reason for hiding this comment

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

As we make a search on channels with product_id in domain I think we can add an index on it, even if we should not have tons of channels.

<odoo>
<data>
<record id="slide_channel_view_form" model="ir.ui.view">
<field name="name">slide.channel.form</field>
Copy link
Contributor

Choose a reason for hiding this comment

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

Guidelines for name: slide.channel.view.form.inherit.sale :) . Xml id is correct, just name.

@@ -122,6 +125,25 @@ def _default_access_token(self):
can_upload = fields.Boolean('Can Upload', compute='_compute_access')
can_publish = fields.Boolean('Can Publish', compute='_compute_access')

@api.depends('custom_slide_id', 'promote_strategy', 'slide_ids.likes',
'slide_ids.total_views', "slide_ids.date_published")
def _compute_promoted_slide_id(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Small note: rebase mismatch, this compute method has been removed.


@api.depends('partner_ids')
def _compute_members_count(self):
for channel in self:
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we could use a read_group (+ sudo to avoid access issues) ?

</field>
</record>

<record id="action_slide_channel_partners" model="ir.actions.act_window">
Copy link
Contributor

Choose a reason for hiding this comment

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

Guidelines: Almost there ! slide_channel_partner_action 💃

@tde-banana-odoo tde-banana-odoo changed the title [ADD] website_slides_sale: allow to sell slide.channel [MERGE][ADD] website_slides_sale: allow to sell slide.channel Feb 11, 2019
@awa-odoo awa-odoo force-pushed the master-slides-sell-courses-awa branch from fbf1b0b to df872be Compare February 11, 2019 16:20
@robodoo robodoo added CI 🤖 Robodoo has seen passing statuses and removed CI 🤖 Robodoo has seen passing statuses labels Feb 11, 2019
Task #1937160

Purpose
=======

This commit adds sale capabilities to a slide.channel. A slide.channel can
now have the 'payment' visibility, that requires a 'product_id' configured
on the channel.

When a customer purchases a product linked to a channel, he is added to the
members of the channel (see slide.channel.partner_ids) when his order is
confirmed.

The feature is added in a new module 'website_sale_slides' that comes as a
bridge auto-installed between 'website_slides' and 'website_sale'.

The final goal is to be able to sell "courses" (see task #1902304).
Task #1937160

Purpose
=======

This commit adds a "Attendees" stat button on the slide.channel form view
showing the members (partner_ids) count.

On click, it redirects to a tree view of slide.channel.partner. The tree
view is configured as editable to be able to administrate members
(add/update/delete).
Purpose
=======

Currently, the slide_channels having a visibility configured as 'invite' could
only add members manually.

This commit adds an 'Invite' button on top of the slide channel form that opens
a wizard allowing to send emails to contacts and also make them members of the
related slide channel.
Purpose
=======

With the new 'website_slides_sale' module users can now sell slide.channels.

This commit adds a setting to let the user know he has to install website_sale
to be able to sell courses / documentation.
@tde-banana-odoo
Copy link
Contributor

@robodoo r+ rebase-merge

@robodoo robodoo added r+ 👌 and removed CI 🤖 Robodoo has seen passing statuses labels Feb 11, 2019
@robodoo
Copy link
Contributor

robodoo commented Feb 11, 2019

Merge method set to rebase and merge, using the PR as merge commit message

@robodoo robodoo added the CI 🤖 Robodoo has seen passing statuses label Feb 11, 2019
robodoo added a commit that referenced this pull request Feb 11, 2019
Purpose of this merge is to be able to sell courses [1] when using the
slides / eLearning platform.

This commit adds sale capabilities on a slide.channel. A slide.channel can
now have the 'payment' visibility, that requires a 'product_id' configured
on the channel.

When a customer purchases a product linked to a channel, he is added to the
members of the channel (see slide.channel.partner_ids) when his order is
confirmed.

This merge is linked to task ID 1937160 and PR #30914. Future tasks will
improve homepage of channels and clearly show public, private and payment
based channels [2].

[1] see task ID 1902304 (main eLearning task) PR #29876;
[2] see task ID 1936153 (new homepage for slides) PR #30770;
@robodoo
Copy link
Contributor

robodoo commented Feb 11, 2019

Merged, thanks!

@robodoo robodoo closed this Feb 11, 2019
@tde-banana-odoo tde-banana-odoo deleted the master-slides-sell-courses-awa branch February 18, 2019 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI 🤖 Robodoo has seen passing statuses RD research & development, internal work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants