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 elearning various fixes qmo #31937

Open
wants to merge 3 commits into
base: saas-12.2
from

Conversation

Projects
None yet
5 participants
@qmo-odoo
Copy link

commented Mar 19, 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 seen 🙂 label Mar 19, 2019

@C3POdoo C3POdoo added the RD label Mar 19, 2019

@qmo-odoo qmo-odoo force-pushed the odoo-dev:saas-12.2-elearning-various-fixes-qmo branch 5 times, most recently from 291171d to b900a4c Mar 19, 2019

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

@dbeguin
Copy link
Contributor

left a comment

Iris é smaul riviouwe !

Aaand, don't forget to squash commit linked to same topic (for ex : quiz fixes and karma gain with quiz)

<span class="mr-5 ml-auto">Dislikes</span>
</span>
</t>
<span t-if="slide.channel_id.allow_comment" class="d-flex pl-3">

This comment has been minimized.

Copy link
@dbeguin

dbeguin Mar 19, 2019

Contributor

I think you can only filter on slide.channel_id.allow_comment in the parent div. and filter again on
slide.channel_id.channel_type == 'documentation' like it's already done. + remove this t-if here.

@@ -191,7 +191,7 @@
t-att-href="'/slides/slide/%s' % (slug(previous_slide)) if previous_slide else '#'">
<i class="fa fa-chevron-left mr-2"></i> Prev
</a>
<t t-set="allow_done_btn" t-value="slide.slide_type in ['infographic', 'presentation', 'document', 'webpage'] and not slide.question_ids and not channel_progress[slide.id].get('completed')"/>
<t t-set="allow_done_btn" t-value="slide.slide_type in ['infographic', 'presentation', 'document', 'webpage', 'video'] and not slide.question_ids and not channel_progress[slide.id].get('completed')"/>

This comment has been minimized.

Copy link
@dbeguin

dbeguin Mar 19, 2019

Contributor

remove space before 'in'

@qmo-odoo qmo-odoo force-pushed the odoo-dev:saas-12.2-elearning-various-fixes-qmo branch from b900a4c to cf9aaa8 Mar 19, 2019

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

@dbeguin

This comment has been minimized.

Copy link
Contributor

commented Mar 19, 2019

Can you apply commit message guidelines (should start with a verb) + add task and PR ids in each commit message ?
And write a quick description of what has been done in this PR + review PR title.
(We will probably make this a rebase-ff)

@qmo-odoo qmo-odoo force-pushed the odoo-dev:saas-12.2-elearning-various-fixes-qmo branch from cf9aaa8 to 938a406 Mar 19, 2019

qmo-odoo added a commit to odoo-dev/odoo that referenced this pull request Mar 19, 2019

[FIX] w_slides: fix various fullscreen issues
This commit provides a few fixes for the fullscreen view:
 - Make external links associated with a slide
   clickable again in fullscreen mode.
 - Hide completion icons if not member
 - Add margin to subquiz icon in header

Task: #1941250
PR: odoo#31937

qmo-odoo added a commit to odoo-dev/odoo that referenced this pull request Mar 19, 2019

[FIX] w_slides: fix karma won and display on quiz
This commit provides a few fixes for quiz feature:
  - Fix the amount of karma won by the user
    in a quiz.
  - correctly display the fact that a subquiz
    was completed by the user instead of just
    looking like a non-completed quiz

Task: #1941250
PR: odoo#31937

qmo-odoo added a commit to odoo-dev/odoo that referenced this pull request Mar 19, 2019

[FIX] w_slides: fix various non-fullscreen view issues
This commit provides a few fixes for the non-fullscreen view:

 - Remove the subquiz link on slides of type 'quiz'
   in the sidebar.
 - Hide completion icons in sidebar if not member
 - Make the 'Set Done' button active for non-completed
   slides of type 'video'.
 - Hide comments and likes/dislikes in the statistics tab in case:
     - training channel: Hide dislikes/likes
     - channel doesn't allow comments: Hide comments
 - Only show the 'embed on your website' option when a slide
   has an embed_code. This will prevent slides of type
   'certification' or 'quiz' to display an empty embed box.

Task: #1941250
PR: odoo#31937

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

@qmo-odoo qmo-odoo force-pushed the odoo-dev:saas-12.2-elearning-various-fixes-qmo branch from 938a406 to 3f6d32a Mar 20, 2019

qmo-odoo added a commit to odoo-dev/odoo that referenced this pull request Mar 20, 2019

[FIX] website_slides: fix various fullscreen issues
This commit provides a few fixes for the fullscreen view:
 - Make external links associated with a slide
   clickable again in fullscreen mode.
 - Hide completion icons if not member
 - Add margin to subquiz icon in header

Task: #1941250
PR: odoo#31937

qmo-odoo added a commit to odoo-dev/odoo that referenced this pull request Mar 20, 2019

[FIX] website_slides: fix quiz issues
This commit provides a few fixes for quiz feature:
  - Fix the amount of karma won by the user
    in a quiz.
  - correctly display the fact that a subquiz
    was completed by the user instead of just
    looking like a non-completed quiz.
  - Going on a previously completed quiz,
    good answers are hightlighted and inputs
    disabled.
  - Effectively put the quiz in readonly mode
    if not a channel member.

Task: #1941250
PR: odoo#31937

qmo-odoo added a commit to odoo-dev/odoo that referenced this pull request Mar 20, 2019

[FIX] website_slides: fix various non-fullscreen view issues
This commit provides a few fixes for the non-fullscreen view:

 - Remove the subquiz link on slides of type 'quiz'
   in the sidebar.
 - Hide completion icons in sidebar if not member
 - Make the 'Set Done' button active for non-completed
   slides of type 'video'.
 - Hide comments and likes/dislikes in the statistics tab in case:
     - training channel: Hide dislikes/likes
     - channel doesn't allow comments: Hide comments
 - Only show the 'embed on your website' option when a slide
   has an embed_code. This will prevent slides of type
   'certification' or 'quiz' to display an empty embed box.

Task: #1941250
PR: odoo#31937

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

@dbeguin
Copy link
Contributor

left a comment

second reviouw

for index_question, question in enumerate(slide.question_ids):
for index_answer, answer in enumerate(question.answer_ids):
quiz_data['questions'][index_question]['answers'][index_answer]['is_correct'] = answer.is_correct
return quiz_data

This comment has been minimized.

Copy link
@dbeguin

dbeguin Mar 20, 2019

Contributor

I would rather add a variable to get once is slide is completed and use it directly in the returned dict:

slide_is_completed = slide.user_membership_id.sudo().completed
quiz_data = {
            'questions': [{
                'id': question.id,
                'question': question.question,
                'answers': [{
                    'id': answer.id,
                    'text_value': answer.text_value,
                    'is_correct': answer.is_correct and slide_is_completed,
                } for answer in question.answer_ids],
            } for question in slide.question_ids],
            'quizAttemptsCount': quiz_info['quiz_attempts_count'],
            'quizKarmaGain': quiz_info['quiz_karma_gain'],
            'quizKarmaWon': quiz_info['quiz_karma_won'],
        }

But.. I don't really understand why this change ? what's the impact if we don't change this at all ?
Is this for rpc security information leak reasons ? -> If yes, write it in the code. Aaaaand, using 'my' method will return false for every answer. So that the hacker is screwed anyway :D

This comment has been minimized.

Copy link
@qmo-odoo

qmo-odoo Mar 21, 2019

Author

JEM thought it was better to not have the key in the dict if the slide wasn't completed by the user. And yes, the change was made to prevent users to cheat the quiz.

This comment has been minimized.

Copy link
@jem-odoo

jem-odoo Mar 21, 2019

Contributor

try None if not completed else answer.is_correct
if None is transformed into undefined, then okay to do that.

<i class="fa fa-times-circle text-danger d-none"></i>
<i class="fa fa-check-circle text-success d-none"></i>
<i t-att-class="'fa fa-check-circle text-success' + (widget.slide.completed &amp;&amp; answer.is_correct ? '' : ' d-none')"></i>

This comment has been minimized.

Copy link
@dbeguin

dbeguin Mar 20, 2019

Contributor

better to use %s then +

This comment has been minimized.

Copy link
@dbeguin

dbeguin Mar 21, 2019

Contributor

Ok my bad, didn't see it was frontend template

@qmo-odoo qmo-odoo force-pushed the odoo-dev:saas-12.2-elearning-various-fixes-qmo branch from 3f6d32a to 3455f31 Mar 21, 2019

qmo-odoo added a commit to odoo-dev/odoo that referenced this pull request Mar 21, 2019

[FIX] website_slides: fix various fullscreen issues
This commit provides a few fixes for the fullscreen view:
 - Make external links associated with a slide
   clickable again in fullscreen mode.
 - Hide completion icons if not member
 - Add margin to subquiz icon in header

Task: #1941250
PR: odoo#31937

qmo-odoo added a commit to odoo-dev/odoo that referenced this pull request Mar 21, 2019

[FIX] website_slides: fix quiz issues
This commit provides a few fixes for quiz feature:
  - Fix the amount of karma won by the user
    in a quiz.
  - correctly display the fact that a subquiz
    was completed by the user instead of just
    looking like a non-completed quiz.
  - Going on a previously completed quiz,
    good answers are hightlighted and inputs
    disabled.
  - Effectively put the quiz in readonly mode
    if not a channel member.

Task: #1941250
PR: odoo#31937

qmo-odoo added a commit to odoo-dev/odoo that referenced this pull request Mar 21, 2019

[FIX] website_slides: fix various non-fullscreen view issues
This commit provides a few fixes for the non-fullscreen view:

 - Remove the subquiz link on slides of type 'quiz'
   in the sidebar.
 - Hide completion icons in sidebar if not member
 - Make the 'Set Done' button active for non-completed
   slides of type 'video'.
 - Hide comments and likes/dislikes in the statistics tab in case:
     - training channel: Hide dislikes/likes
     - channel doesn't allow comments: Hide comments
 - Only show the 'embed on your website' option when a slide
   has an embed_code. This will prevent slides of type
   'certification' or 'quiz' to display an empty embed box.

Task: #1941250
PR: odoo#31937

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

@qmo-odoo qmo-odoo force-pushed the odoo-dev:saas-12.2-elearning-various-fixes-qmo branch from 3455f31 to 2807272 Mar 21, 2019

qmo-odoo added some commits Mar 18, 2019

[FIX] website_slides[_survey]: fix various fullscreen issues
This commit provides a few fixes for the fullscreen view:
 - Make external links associated with a slide
   clickable again in fullscreen mode.
 - Hide completion icons if not member
 - Add margin to subquiz icon in header
 - Add dedicated button to exit the fullscreen
 - Change "Pass certification" button to
   "Test Certification" if not member of
   the channel

Task: #1941250
PR: #31937

This commit changes the "Pass Certification" button to "Test certification"
[FIX] website_slides: fix quiz issues
This commit provides a few fixes for quiz feature:
  - Fix the amount of karma won by the user
    in a quiz.
  - correctly display the fact that a subquiz
    was completed by the user instead of just
    looking like a non-completed quiz.
  - Going on a previously completed quiz,
    good answers are hightlighted and inputs
    disabled.
  - Effectively put the quiz in readonly mode
    if not a channel member.

Task: #1941250
PR: #31937
[FIX] website_slides: fix various non-fullscreen view issues
This commit provides a few fixes for the non-fullscreen view:

 - Remove the subquiz link on slides of type 'quiz'
   in the sidebar.
 - Hide completion icons in sidebar if not member
 - Make the 'Set Done' button active for non-completed
   slides of type 'video'.
 - Hide comments and likes/dislikes in the statistics tab in case:
     - training channel: Hide dislikes/likes
     - channel doesn't allow comments: Hide comments
 - Only show the 'embed on your website' option when a slide
   has an embed_code. This will prevent slides of type
   'certification' or 'quiz' to display an empty embed box.
 - Hide set done and fullscreen buttons if the channel is
   of type 'documentation'

Task: #1941250
PR: #31937

@qmo-odoo qmo-odoo force-pushed the odoo-dev:saas-12.2-elearning-various-fixes-qmo branch from 2807272 to 1b1fd75 Mar 21, 2019

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

tde-banana-odoo added a commit to odoo-dev/odoo that referenced this pull request Mar 21, 2019

[FIX] website_slides[_survey]: fix various fullscreen issues
This commit provides a few fixes for the fullscreen view:
 - Make external links associated with a slide
   clickable again in fullscreen mode.
 - Hide completion icons if not member
 - Add margin to subquiz icon in header
 - Add dedicated button to exit the fullscreen
 - Change "Pass certification" button to
   "Test Certification" if not member of
   the channel

Task: #1941250
PR: odoo#31937

This commit changes the "Pass Certification" button to "Test certification"

tde-banana-odoo added a commit to odoo-dev/odoo that referenced this pull request Mar 21, 2019

[FIX] website_slides: fix quiz issues
This commit provides a few fixes for quiz feature:
  - Fix the amount of karma won by the user
    in a quiz.
  - correctly display the fact that a subquiz
    was completed by the user instead of just
    looking like a non-completed quiz.
  - Going on a previously completed quiz,
    good answers are hightlighted and inputs
    disabled.
  - Effectively put the quiz in readonly mode
    if not a channel member.

Task: #1941250
PR: odoo#31937

tde-banana-odoo added a commit to odoo-dev/odoo that referenced this pull request Mar 21, 2019

[FIX] website_slides: fix various non-fullscreen view issues
This commit provides a few fixes for the non-fullscreen view:

 - Remove the subquiz link on slides of type 'quiz'
   in the sidebar.
 - Hide completion icons in sidebar if not member
 - Make the 'Set Done' button active for non-completed
   slides of type 'video'.
 - Hide comments and likes/dislikes in the statistics tab in case:
     - training channel: Hide dislikes/likes
     - channel doesn't allow comments: Hide comments
 - Only show the 'embed on your website' option when a slide
   has an embed_code. This will prevent slides of type
   'certification' or 'quiz' to display an empty embed box.
 - Hide set done and fullscreen buttons if the channel is
   of type 'documentation'

Task: #1941250
PR: odoo#31937
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.