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] website_slides: re-order templates and finalize model cleaning #31272

Closed
wants to merge 6 commits into
base: saas-12.2
from

Conversation

Projects
None yet
4 participants
@tde-banana-odoo
Copy link
Contributor

tde-banana-odoo commented Feb 20, 2019

Purpose of this merge is to perform a quick linting after having merged
several tasks and PR adding all requested eLearning features. Templates
are split for better understanding now that main templates for homepage,
course and lessons are in.

A model cleaning is performed as well as a-bit-late additions to stick
to specifications, notably splitting short and long description fields
on course.

See sub commits for more details. Merge linked to task ID 1941250 and
PR #31272.

@robodoo robodoo added the seen 🙂 label Feb 20, 2019

@tde-banana-odoo tde-banana-odoo force-pushed the odoo-dev:saas-12.2-elearning-fiximp-tde branch Feb 20, 2019

@C3POdoo C3POdoo added the RD label Feb 20, 2019

@tde-banana-odoo tde-banana-odoo force-pushed the odoo-dev:saas-12.2-elearning-fiximp-tde branch 2 times, most recently Feb 20, 2019

@tde-banana-odoo tde-banana-odoo force-pushed the odoo-dev:saas-12.2-elearning-fiximp-tde branch Feb 20, 2019

[IMP] website_slides: perform some code linting and re-order frontend…
… templates

Purpose is to perform some linting in code. No functional change should
occur with this commit, only some code move. Diff may appear huge but
it is just because some files are split.

This cleaning allow to better understand code and templates architecture
now that eLearning feature has landed in Odoo Community. It will ease future
additions, modifications and fixes.

Containing :
 * [REF] make some internal slide-related methods private as they are not
   intended to be called by outside world;
 * [MOV] new channel invite wizard in wizard folder;
 * [MOV] split frontend templates in some files to better locate code. There is
   now files for homepage, course, lesson, embed and utils in addition to
   slide-related profile template;
 * [REM] dead (unused) templates;

Commit linked to task ID 1941250 and PR #31272.

@tde-banana-odoo tde-banana-odoo force-pushed the odoo-dev:saas-12.2-elearning-fiximp-tde branch Feb 20, 2019

tde-banana-odoo added some commits Feb 18, 2019

[REF] website_slides: use mixin.image on channel and slide models
This recently introduced mixin allows to hide complexity of defining, using
and resizing images. Let us use it in channel and image model.

Commit linked to task ID 1941250 and PR #31272.
[REF] website_slide: rename statistics fields on channel and category…
… models

Purpose is to reduce model naming complexity. Currently you have: slide_type
<type>, and related statistics on channel and category nbr_<type>s with a
trailing and error-prone s. Let us remove the trailing s to reduce complexity
of code reading. Some code cleaning in statistics computation is also done.

Commit linked to task ID 1941250 and PR #31272.
[FIX] website_slides: clean some model-related parameters
Quiz questions and answers should be translatable as they are intended
for end users and are displayed in courses frontend. Some required fields
are also missing. Some docstrings are cleaned.

Commit linked to task ID 1941250 and PR #31272.
[MOV] website_slides: split channel-type specific display in course i…
…nto sub templates

It will ease maintenance and polish of those templates.

Commit linked to task ID 1941250 and PR #31272.

@tde-banana-odoo tde-banana-odoo force-pushed the odoo-dev:saas-12.2-elearning-fiximp-tde branch Feb 20, 2019

[REF] website_slide: split short from long description in channel model
Purpose of this commit is to provide a better way to display channel information.
Description is split into two fields. A text one, holding the short description
and used in channel header. A long html one, holding more details course
description.

Commit linked to task ID 1941250 and PR #31272.

@tde-banana-odoo tde-banana-odoo force-pushed the odoo-dev:saas-12.2-elearning-fiximp-tde branch to 22fa2cf Feb 20, 2019

@tde-banana-odoo tde-banana-odoo changed the title Saas 12.2 elearning fiximp tde [MERGE] website_slides: re-order templates and finalize model cleaning Feb 20, 2019

@tde-banana-odoo

This comment has been minimized.

Copy link
Contributor Author

tde-banana-odoo commented Feb 20, 2019

@robodoo r+ rebase-merge

@robodoo robodoo added the r+ 👌 label Feb 20, 2019

@robodoo

This comment has been minimized.

Copy link
Contributor

robodoo commented Feb 20, 2019

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

robodoo pushed a commit that referenced this pull request Feb 20, 2019

[IMP] website_slides: perform some code linting and re-order frontend…
… templates

Purpose is to perform some linting in code. No functional change should
occur with this commit, only some code move. Diff may appear huge but
it is just because some files are split.

This cleaning allow to better understand code and templates architecture
now that eLearning feature has landed in Odoo Community. It will ease future
additions, modifications and fixes.

Containing :
 * [REF] make some internal slide-related methods private as they are not
   intended to be called by outside world;
 * [MOV] new channel invite wizard in wizard folder;
 * [MOV] split frontend templates in some files to better locate code. There is
   now files for homepage, course, lesson, embed and utils in addition to
   slide-related profile template;
 * [REM] dead (unused) templates;

Commit linked to task ID 1941250 and PR #31272.

robodoo pushed a commit that referenced this pull request Feb 20, 2019

[MOV] website_slides: split channel-type specific display in course i…
…nto sub templates

It will ease maintenance and polish of those templates.

Commit linked to task ID 1941250 and PR #31272.

robodoo pushed a commit that referenced this pull request Feb 20, 2019

[FIX] website_slides: clean some model-related parameters
Quiz questions and answers should be translatable as they are intended
for end users and are displayed in courses frontend. Some required fields
are also missing. Some docstrings are cleaned.

Commit linked to task ID 1941250 and PR #31272.

robodoo pushed a commit that referenced this pull request Feb 20, 2019

[REF] website_slides: use mixin.image on channel and slide models
This recently introduced mixin allows to hide complexity of defining, using
and resizing images. Let us use it in channel and image model.

Commit linked to task ID 1941250 and PR #31272.

robodoo pushed a commit that referenced this pull request Feb 20, 2019

[REF] website_slide: rename statistics fields on channel and category…
… models

Purpose is to reduce model naming complexity. Currently you have: slide_type
<type>, and related statistics on channel and category nbr_<type>s with a
trailing and error-prone s. Let us remove the trailing s to reduce complexity
of code reading. Some code cleaning in statistics computation is also done.

Commit linked to task ID 1941250 and PR #31272.

robodoo pushed a commit that referenced this pull request Feb 20, 2019

[REF] website_slide: split short from long description in channel model
Purpose of this commit is to provide a better way to display channel information.
Description is split into two fields. A text one, holding the short description
and used in channel header. A long html one, holding more details course
description.

Commit linked to task ID 1941250 and PR #31272.

robodoo added a commit that referenced this pull request Feb 20, 2019

[MERGE] website_slides: re-order templates and finalize model cleaning
Purpose of this merge is to perform a quick linting after having merged
several tasks and PR adding all requested eLearning features. Templates
are split for better understanding now that main templates for homepage,
course and lessons are in.

A model cleaning is performed as well as a-bit-late additions to stick
to specifications, notably splitting short and long description fields
on course.

See sub commits for more details. Merge linked to task ID 1941250 and
PR #31272.
@@ -119,7 +119,7 @@ def _build_channel_domain(self, base_domain, slide_type=None, **post):
domain = expression.AND([domain, [('tag_ids', 'in', tags.ids)]])

if slide_type and 'nbr_%ss' % slide_type in request.env['slide.channel']:

This comment has been minimized.

@KangOl

KangOl Feb 20, 2019

Contributor

extra s

@KangOl

This comment has been minimized.

Copy link
Contributor

KangOl commented Feb 20, 2019

WTF?

The very first PR on this stable branch change the database schema?

@tde-banana-odoo

This comment has been minimized.

Copy link
Contributor Author

tde-banana-odoo commented Feb 20, 2019

Thanks for noticing the typo, I forgot to commit it since it comes from another branch that was waiting QMO's merge. It will be fixed asap within another commit, as other people depend on content from this PR and I prefer not re-stage it.

Concerning fields I understand you feel a bit angry about it. Be sure I will tell PO and functional people developers prefer having clear specifications a sufficient amount of time before a deadline. Let us hope post-merge specifications won't be too numerous.

@robodoo robodoo added merged 🎉 and removed merging 👷 labels Feb 20, 2019

@robodoo

This comment has been minimized.

Copy link
Contributor

robodoo commented Feb 20, 2019

Merged, thanks!

@robodoo robodoo closed this Feb 20, 2019

@tde-banana-odoo tde-banana-odoo deleted the odoo-dev:saas-12.2-elearning-fiximp-tde branch Feb 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.