[IMP] survey: improve session code generation #164687
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
flch-odoo
reviewed
Jun 6, 2024
addons/survey/tests/test_survey.py
Outdated
Comment on lines
273
to
275
| self.assertTrue(all( | ||
| bool(session_code) for session_code in surveys.mapped('session_code') | ||
| )) |
There was a problem hiding this comment.
Suggested change
| self.assertTrue(all( | |
| bool(session_code) for session_code in surveys.mapped('session_code') | |
| )) | |
| self.assertNotIn(False, surveys.mapped('session_code')) |
Comment on lines
+270
to
+272
| surveys = self.env['survey.survey'].create([{ | ||
| 'title': f'Survey {i}' | ||
| } for i in range(10)]) |
There was a problem hiding this comment.
This is a nice result! I think we can now simplify a few tests that had split create because of the codes (or I had to manually set to fix tours).
| @@ -317,6 +297,17 @@ def _compute_session_question_answer_count(self): | |||
| )[0] | |||
| survey.session_question_answer_count = answer_count | |||
|
|
|||
| @api.depends('access_token') | |||
| def _compute_session_code(self): | |||
| self.flush_model(['session_code']) | |||
There was a problem hiding this comment.
Isn't this automatically done in _fetch_query since session_code is in the requested fields?
And if not, shouldn't this rather be in _generate_session_codes before the search_read?
Comment on lines
1285
to
1290
| code_candidates -= {colliding_code['session_code'] for colliding_code in colliding_codes} | ||
| code_candidates -= excluded_codes | ||
| if code_candidates: | ||
| session_codes += list(code_candidates)[:code_count] | ||
| if len(session_codes) == code_count: | ||
| return session_codes |
There was a problem hiding this comment.
Keeping session_codes as a set until returning and with an additional step outside the loop
unavailable_codes = excluded_codes + {colliding_code['session_code'] for colliding_code in colliding_codes}we could have the slightly shorter/simpler
Suggested change
| code_candidates -= {colliding_code['session_code'] for colliding_code in colliding_codes} | |
| code_candidates -= excluded_codes | |
| if code_candidates: | |
| session_codes += list(code_candidates)[:code_count] | |
| if len(session_codes) == code_count: | |
| return session_codes | |
| session_codes += code_candidates - unavailable_codes | |
| if len(session_codes) >= code_count: | |
| return list(session_codes)[:code_count] |
| excluded_codes = excluded_codes or set() | ||
| colliding_codes = self.sudo().search_read( | ||
| [('session_code', '!=', False)], | ||
| ['session_code'] |
There was a problem hiding this comment.
It doesn't cost much to have this as well
Suggested change
| ['session_code'] | |
| ['session_code'], | |
| load='', |
| @@ -317,6 +297,17 @@ def _compute_session_question_answer_count(self): | |||
| )[0] | |||
| survey.session_question_answer_count = answer_count | |||
|
|
|||
| @api.depends('access_token') | |||
There was a problem hiding this comment.
This seems to work as fake-dependency trigger. I'm guessing because it's required and therefore present when precomputing 🤷 ?
Suggested change
| @api.depends('access_token') | |
| @api.depends('title') |
awa-odoo
force-pushed
the
master-survey-session-code-rework-awa
branch
from
a9059f8
to
0421915
Compare
The current session code generation for surveys is a bit messy: - It runs multiple times for a create-multi syntax - It has poor performance with a search within a loop - It can fail non-deterministically This commit reworks the code generation in favor of a precomputed stored field with the compute method generating all the necessary codes at once. Task-3916147 Runbot-error-57541
awa-odoo
force-pushed
the
master-survey-session-code-rework-awa
branch
from
0421915
to
7ef8205
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The current session code generation for surveys is a bit messy:
This commit reworks the code generation in favor of a precomputed stored field with the compute method generating all the necessary codes at once.
Task-3916147
Runbot-error-57541