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

Saas 12.2 fixes jem #31843

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

Conversation

Projects
None yet
6 participants
@jem-odoo
Copy link
Contributor

jem-odoo commented Mar 14, 2019

Description of the issue/feature this PR addresses:

Current behavior before PR:

Desired behavior after PR is merged:

--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

@robodoo robodoo added the CI 🤖 label Mar 14, 2019

@C3POdoo C3POdoo added the RD label Mar 14, 2019

@awa-odoo
Copy link
Contributor

awa-odoo left a comment

An architectural question about "_slide_set_completed_check" and some smaller remarks/questions about the rest.
Let's make elearning great again.

Show resolved Hide resolved addons/website_slides_survey/models/slide_slide.py Outdated
Show resolved Hide resolved addons/website_slides_survey/models/slide_slide.py Outdated
Show resolved Hide resolved addons/website_slides_survey/models/slide_slide.py Outdated
Show resolved Hide resolved addons/website_slides/controllers/main.py Outdated
Show resolved Hide resolved addons/website_slides/static/src/js/slides_course_fullscreen_player.js Outdated
Show resolved Hide resolved addons/website_slides/models/slide_slide.py
Show resolved Hide resolved addons/website_slides/views/website_slides_templates_lesson_embed.xml
Show resolved Hide resolved addons/website_slides/static/src/js/slides_course_fullscreen_player.js
Show resolved Hide resolved addons/website_slides/static/src/js/slides_course_fullscreen_player.js
Show resolved Hide resolved addons/website_slides/static/src/js/slides_course_fullscreen_player.js

@jem-odoo jem-odoo force-pushed the odoo-dev:saas-12.2-fixes-jem branch Mar 14, 2019

@robodoo robodoo removed the CI 🤖 label Mar 14, 2019

@jem-odoo jem-odoo force-pushed the odoo-dev:saas-12.2-fixes-jem branch 2 times, most recently to 02b6317 Mar 14, 2019

@robodoo robodoo added CI 🤖 and removed CI 🤖 labels Mar 14, 2019

channel.valid_channel_partner_ids = result.get(channel.id, False)
channel.is_member = self.env.user.partner_id.id in channel.valid_channel_partner_ids if channel.valid_channel_partner_ids else False
valid_channel_partner_ids = result.get(channel.id, False)
channel.is_member = self.env.user.partner_id.id in valid_channel_partner_ids if valid_channel_partner_ids else False

This comment has been minimized.

@awa-odoo

awa-odoo Mar 15, 2019

Contributor

Could even be channel.is_member = self.env.user.partner_id.id in result.get(channel.id, []) to make it a lot shorter?

@@ -64,6 +64,9 @@ def _set_viewed_slide(self, slide, quiz_attempts_inc=False):
return True

def _set_completed_slide(self, slide, quiz_attempts_inc=False):
# quiz used its specif mecanism to be mark as done

This comment has been minimized.

@awa-odoo

awa-odoo Mar 15, 2019

Contributor

"quiz use their specific mechanism to be marked as done"

@@ -53,11 +53,10 @@ def _get_slide_detail(self, slide):

# Utils
# ---------------------------------------------------
def _slide_set_completed_check(self, slide):
result = super(WebsiteSlides, self)._slide_set_completed_check(slide)
def _set_completed_slide(self, slide, quiz_attempts_inc=False):
if slide.slide_type == 'certification':
raise werkzeug.exceptions.Forbidden(_("Certification slide are completed when the survey is succeed."))

This comment has been minimized.

@awa-odoo

awa-odoo Mar 15, 2019

Contributor

"slides" and "succeeded"

@jem-odoo jem-odoo force-pushed the odoo-dev:saas-12.2-fixes-jem branch from 5229ee1 to 972907b Mar 15, 2019

@robodoo robodoo removed the CI 🤖 label Mar 15, 2019

@jem-odoo jem-odoo force-pushed the odoo-dev:saas-12.2-fixes-jem branch 3 times, most recently from 9497c5b to 67c0493 Mar 15, 2019

@robodoo robodoo added the CI 🤖 label Mar 15, 2019

@jem-odoo jem-odoo force-pushed the odoo-dev:saas-12.2-fixes-jem branch from 67c0493 to 8d9d1f7 Mar 15, 2019

@robodoo robodoo added CI 🤖 and removed CI 🤖 labels Mar 15, 2019

awa-odoo and others added some commits Feb 26, 2019

[IMP] survey: add a download button at the end of certification process
Purpose
=======

When the user successfully takes a certification, he can download the certification
document using a new "Download certification" button.
(In addition to receiving an email containing the certification)

Also cleaned a bit the way certifications are downloaded.

Task-1941250
[FIX] website_slides: avoid access error when submitting quiz
Purpose
=======

The quiz_submit method and the action to set the quiz done both use
the slide_partner relation that has to be accessed in sudo mode.

Task-1941250
[FIX] website_slides: styling fullscreen mode
Since the fullscreen widget has been revamped, it needed
to be pixel perfect. This commit cleans the CSS and DOM
structure in order to optimize and sublime it.

Task-1941250
[FIX] website_slides_survey: certification slide url
Before this commit, in fullscreen mode, clicking on the "pass certification"
button would redirect the user to the "non-fullscreen" view.
After this commit, the button directly redirects the user to the survey page.

To avoid generating user_input of survey (test entry or not) for
certification slides in fullscreen mode (table of content), we decided
to create those user_input lazily: we redirect the user to a route that
create (or reuse) the correct user_input.

Task-1946511
[IMP] website_slides: embedded player restyling
The style and DOM structure of the embeded player (used
to display PDF file) is old, and it needed a little refresh
to be adapt to bootstrap4 and modern styling.

Task-1941250
[FIX] website_slides: auto set done successfull quizz
For now, in no fullscreen mode, if the quiz is succeed, the
user progress is not updated because the quiz is not set
to done.
This commit implements this call to mark slide as done and
green the check bullet.
We decided to unify the submit RPC call with the done one,
so a quiz (or a slide with questions) can only be done when
submitting answers. As consequence, we need to prevent some
slide type to use the /set_completed route (quiz and certif).

Task-1941250
[FIX] website_slides_survey: go to survey if only one slide certifica…
…tion

Purpose of this commit:
  In case there is only one slide in the course and that unique slide
  is of type certification, clicking on the link redirects the user
  to the survey page instead of the fullscreen view

Task-1946511
[FIX] website_slides: prevent to mark a certification as done
When a certification slide is display in no fullscreen
mode, the done button is clickable. Same applies to quiz
slide.
This commit prevents user to manually mark a slide as done
for slide type that requires an other action (submit answer,
pass survey, ...).

Task-1941250
@jem-odoo

This comment has been minimized.

Copy link
Contributor Author

jem-odoo commented Mar 15, 2019

@robodoo rebase-ff r+ ma biche !

@robodoo robodoo added the r+ 👌 label Mar 15, 2019

@robodoo

This comment has been minimized.

Copy link
Contributor

robodoo commented Mar 15, 2019

Merge method set to rebase and fast-forward

@robodoo robodoo added the CI 🤖 label Mar 15, 2019

robodoo pushed a commit that referenced this pull request Mar 15, 2019

[FIX] website_slides_survey: go to survey if only one slide certifica…
…tion

Purpose of this commit:
  In case there is only one slide in the course and that unique slide
  is of type certification, clicking on the link redirects the user
  to the survey page instead of the fullscreen view

Task-1946511

closes #31843

Signed-off-by: Jérome Maes (jem) <jem@openerp.com>

@robodoo robodoo added merging 👷 and removed r+ 👌 labels Mar 15, 2019

@robodoo

This comment has been minimized.

Copy link
Contributor

robodoo commented Mar 15, 2019

Staging failed: ci/runbot on ca28fa4f79037bc7eab2011b19ec2746222b0bc1 (view more at http://runbot.odoo.com/runbot/build/480602)

@jem-odoo

This comment has been minimized.

Copy link
Contributor Author

jem-odoo commented Mar 15, 2019

@robodoo retry please

@robodoo

This comment has been minimized.

Copy link
Contributor

robodoo commented Mar 16, 2019

Staging failed: ci/runbot on 65e31e421de2714b12aaf9d14add54383d6f3dfd (view more at http://runbot.odoo.com/runbot/build/480720)

@robodoo robodoo added error 🙅 and removed merging 👷 labels Mar 16, 2019

@jem-odoo

This comment has been minimized.

Copy link
Contributor Author

jem-odoo commented Mar 16, 2019

@robodoo retry please, I know you are not on holidays :p

@robodoo

This comment has been minimized.

Copy link
Contributor

robodoo commented Mar 16, 2019

Merged, thanks!

@robodoo robodoo closed this Mar 16, 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.