Skip to content

Conversation

@nd-dew
Copy link
Contributor

@nd-dew nd-dew commented Dec 5, 2024

[FIX] survey: allow survey print without answers

Issue

Previously, printing a survey with access mode set to "invited people only"
required an existing survey.user_input record.
Without it, the print route failed silently and redirected to the homepage.

FIX

This commit allows surveys to be printed without answers.
It introduces an option to bypass the "token_required" validity check
by allowing ensure_token='survey_only'. This skips token validation
specifically for the print route, while preserving it for the start route.

Reproduce

  • -i survey
  • create new Survey, with "Access Mode": "invited people only"
  • click "Print" BUG -> taken to the odoo main page, instead of empty printed survey

opw-4292331



LEGACY notes BELOW


[FIX] survey: allow survey print without answers

When we set access mode to token, we need
to have existing registration survey.user_input to print the survey.

After this commit existing answer is not required anymore

Reproduce

  • -i survey
  • create new Survey, with "Access Mode": "invited people only"
  • click "Print" BUG -> taken to the odoo main page, instead of empty printed survey

opw-4292331


Note on other solutions

Initially explored approach was just to add a set of dummy/test answers

[FIX] survey: create empty answer when token access set

... to allow general print of the survey.

When we set access mode to token, we need
to have existing registration survey.user_input to print the survey.

This commit suggest solution similar to the one existing in survey_test,
so if we create test answer to use it.

Reproduce

  • -i survey
  • create new Survey, with "Access Mode": "invited people only"
  • click "Print" BUG -> taken to the odoo main page, instead of empty printed survey

Def

What I refer to as "General print" of the survey is a request to print survey without answer_token.
This is currently possible to request from the form view of the survey.survey

image

opw-4292331

@robodoo
Copy link
Contributor

robodoo commented Dec 5, 2024

Pull request status dashboard

@nd-dew nd-dew force-pushed the 17.0-opw-4292331-general_survey_print_with_token_access-pian branch from 75c41ab to 1921e39 Compare December 5, 2024 12:32
@C3POdoo C3POdoo added the OE the report is linked to a support ticket (opw-...) label Dec 5, 2024
@nd-dew nd-dew marked this pull request as ready for review December 5, 2024 14:54
@nd-dew nd-dew marked this pull request as draft December 5, 2024 14:54
@amdi-odoo
Copy link
Contributor

Hello 👋 Passing by!

In the survey controller _check_validity method we have this condition.
In our case, the answer_token is None making the answer_sudo an empty recordset and we have the access_mode to 'token', the condition is evaluated to True which throws the token_required and the user is redirected.

I originally thought of changing the condition to use the ensure_token parameter like so:

:param ensure_token: whether user input existence based on given access token
  should be enforced or not, depending on the route requesting a token or
  allowing external world calls;
if not answer_sudo and ensure_token and survey_sudo.access_mode == 'token':

Because in the survey/print route the ensure_token is set to False.

However this won't work because the survey_start route is also setting this ensure_token to False.
So if we change the condition, anyone could start the survey even if the access_mode is 'token'...

@nd-dew 's approach here is to create a dummy test user.input record so that the answer_sudo isn't empty and the token_required error isn't thrown. I think it effectively solves the issue without messing with the current access implementation.

@nd-dew nd-dew marked this pull request as ready for review December 10, 2024 10:31
@nd-dew nd-dew marked this pull request as draft December 10, 2024 10:31
@nd-dew nd-dew force-pushed the 17.0-opw-4292331-general_survey_print_with_token_access-pian branch from 1921e39 to 8fad61d Compare December 10, 2024 12:31
@nd-dew
Copy link
Contributor Author

nd-dew commented Dec 10, 2024

Added test

@nd-dew nd-dew marked this pull request as ready for review December 10, 2024 12:31
@C3POdoo C3POdoo requested a review from a team December 10, 2024 12:34
@nd-dew
Copy link
Contributor Author

nd-dew commented Jan 7, 2025

Temporary Reverted Previous FIX -> Experiment: alter _check_validity with a context key

@nd-dew nd-dew force-pushed the 17.0-opw-4292331-general_survey_print_with_token_access-pian branch from c6d623e to f638799 Compare January 7, 2025 10:28
@nd-dew
Copy link
Contributor Author

nd-dew commented Feb 3, 2025

@std-odoo this seem to work, but isn't that too general of a change ?

@std-odoo
Copy link
Contributor

std-odoo commented Feb 5, 2025

@nd-dew Hi

We talk about that a long time ago, but iirc, the idea was to have 2 values for ensure_token no ?

image

@nd-dew
Copy link
Contributor Author

nd-dew commented Feb 10, 2025

Hey, I see I misunderstood your approach in there, let me introduce ensure_token set to allow_no_answer_token_mode for the survey_print route

@nd-dew nd-dew force-pushed the 17.0-opw-4292331-general_survey_print_with_token_access-pian branch 2 times, most recently from 278b8d5 to 3055e87 Compare February 10, 2025 13:15
@nd-dew
Copy link
Contributor Author

nd-dew commented Feb 10, 2025

Seems to work fine, squash & clean

@nd-dew nd-dew force-pushed the 17.0-opw-4292331-general_survey_print_with_token_access-pian branch from 3055e87 to 93d4415 Compare February 10, 2025 16:27
Copy link
Contributor

@std-odoo std-odoo left a comment

Choose a reason for hiding this comment

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

@nd-dew Hi :)

Thanks for the update, I'm ok with the approach :)

cc @amdi-odoo @flch-odoo

@nd-dew
Copy link
Contributor Author

nd-dew commented Feb 27, 2025

@flch-odoo what do you think about this approach ?

@nd-dew nd-dew force-pushed the 17.0-opw-4292331-general_survey_print_with_token_access-pian branch from 93d4415 to 3a63db5 Compare March 28, 2025 14:57
@nd-dew nd-dew changed the title [FIX] survey: create empty answer when token access set [FIX] survey: allow survey print without answers Mar 28, 2025
@nd-dew
Copy link
Contributor Author

nd-dew commented Mar 28, 2025

Runbot failure on seemingly unrelated tests -> r-basing

@flch-odoo tried to address your comments does the PR look more sensible now?

@nd-dew nd-dew force-pushed the 17.0-opw-4292331-general_survey_print_with_token_access-pian branch from 3a63db5 to a68b223 Compare March 28, 2025 17:05
@nd-dew nd-dew requested a review from flch-odoo April 28, 2025 08:29
Copy link
Contributor

@flch-odoo flch-odoo left a comment

Choose a reason for hiding this comment

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

Thank you for the update, @nd-dew !
I have a few questions to simplify further.

Cheers!

@nd-dew nd-dew force-pushed the 17.0-opw-4292331-general_survey_print_with_token_access-pian branch 2 times, most recently from 363c3ba to 5e9ec4b Compare May 22, 2025 11:32
@nd-dew
Copy link
Contributor Author

nd-dew commented May 22, 2025

Update

  • printing a survey with no questions and no answers (displays "survey is empty")
  • printing a survey with a question but no answers -> shows just questions
  • printing a survey with existing answers (but no answer_token) — question is shown, answer is not
  • printing a survey with answer_token — both question and answer are shown (most normal/classic use-case)

and rebased

@nd-dew nd-dew force-pushed the 17.0-opw-4292331-general_survey_print_with_token_access-pian branch 2 times, most recently from 5aac5af to c46e8f1 Compare May 22, 2025 15:28
Issue
---
Previously, printing a survey with access mode set to "invited people only"
required an existing `survey.user_input` record.
Without it, the print route failed silently and redirected to the homepage.

FIX
---
This commit allows surveys to be printed without answers.
It introduces an option to bypass the "token_required" validity check
by allowing `ensure_token='survey_only'`. This skips token validation
specifically for the print route, while preserving it for the start route.

Reproduce
---
- -i survey
- create new Survey, with "Access Mode": "invited people only"
- click "Print" BUG -> taken to the odoo main page, instead of empty printed survey

opw-4292331
@nd-dew nd-dew force-pushed the 17.0-opw-4292331-general_survey_print_with_token_access-pian branch from c46e8f1 to 90d14a9 Compare May 23, 2025 09:03
@flch-odoo
Copy link
Contributor

Have a nice week-end, @nd-dew !

@robodoo r+

robodoo pushed a commit that referenced this pull request May 23, 2025
Issue
---
Previously, printing a survey with access mode set to "invited people only"
required an existing `survey.user_input` record.
Without it, the print route failed silently and redirected to the homepage.

FIX
---
This commit allows surveys to be printed without answers.
It introduces an option to bypass the "token_required" validity check
by allowing `ensure_token='survey_only'`. This skips token validation
specifically for the print route, while preserving it for the start route.

Reproduce
---
- -i survey
- create new Survey, with "Access Mode": "invited people only"
- click "Print" BUG -> taken to the odoo main page, instead of empty printed survey

opw-4292331

closes #189738

Signed-off-by: Florian Charlier (flch) <flch@odoo.com>
@robodoo robodoo closed this May 23, 2025
@fw-bot fw-bot deleted the 17.0-opw-4292331-general_survey_print_with_token_access-pian branch May 30, 2025 17:37
pish-odoo added a commit to odoo-dev/odoo that referenced this pull request Jun 27, 2025
Steps to reproduce:
1. Install appraisals
2. appraisals > configuration menu > settings > check '360 feedback'
3. appraisals > configuration > 360 feedback
4. Open any survey with 'invite people only' access mode or
5. Create a survey with no questions and 'invite people only' access mode.
6. Print it.

Issue:
Clicking the print button redirects to the homepage instead of
opening the survey template.

cause:
The access check incorrectly redirects users
additionally this pr was not backported that skips token validation
specifically for the print route to redirect on print page even when no
questions are added.

odoo#189738

Solution:
Backport the pr : odoo#189738.

opw:4878800
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

OE the report is linked to a support ticket (opw-...)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants