Prohibit access on Recordset attributes if more than 1 record #7
Conversation
This file contains hidden or 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
|
Totally agree on the purpose. It will avoid unexpected behaviors if developers think there is one record but in fact there are several in a recordset. |
|
Seems smart to me, but maybe it'd be better to raise a specific error or use an assertion (not sure what @rco-odoo's policy is on that)? Although I'm a frequent user of them, in my experience The assertion could even be moved to a recordset method of some sort, I'd guess (it'd just perform the assertion and return |
|
I've to set-up a VM and run the tests because it seems it breaks some models in For the kind of errors, we could do whatever is the recommendation of @odoo, np. |
florentx
changed the title
|
The build is still red, but it seems no longer related to this PR. |
florentx
changed the title
There was a problem hiding this comment.
I thing this one have to work has is. Have too look to event.py to be sure
There was a problem hiding this comment.
I confirm it is intended to work this way
There was a problem hiding this comment.
Indeed, "record.id" should only work if len(record) <= 1.
|
I will merge your pull request and the one from @nbessi, and rework a bit the result. |
|
good, thanks |
apineux
pushed a commit
that referenced
this pull request
Feb 24, 2015
Lastest updates from Odoo/8.0
mba-odoo
pushed a commit
that referenced
this pull request
Jul 19, 2017
… invoiced based ondelivered qty. Sometimes users expect more flexibility and improve the create invoice wizard (remove the complexity) commit #7: created horizontal options for main menu and 2/3 are operational. commit #8: adding option 'all lines not yet invoicd' [IMP]Paymentlinewidget: display popup on dynamic position based on requirement [IMP]improve the code [IMP]improve the code [IMP]improve the code commit#14 [IMP]improve the code resolved test cases [IMP] sale: updated as per new specs-displayed correct deduct down payment amount, removed payment widget. [IMP]improve the code and view review changes review changes#2 [REV]Revert rebase chnages
cammarosano
added a commit
that referenced
this pull request
Dec 28, 2022
Issue #1: pasting image/video URL as link or text Before this commit, pasting an image or video url as simple text was failing, and pasting it as a link was undoing the last modification before it. This happens because Powerbox.preValidate() reverts one history step before calling the callback attached to the Powerbox command. When pasting the url as simple text, the result was deleting the already inserted text. In case of pasting the url as a link, the callback to this command already performs a historyUndo(), and reverting history one step further leads to loss of previous modifications. In order to skip the call to preValidate, this commit adds a "shouldPreValidate" method (returning "false") to the command passed to Powerbox.open(). Such mechanism was introduced by commit odoo@a9c363c. Issue #2: transforming image/video URL pasted as text with space After pasting a URL as text at the end of a line, there's no way to type more text in that line (separated from the link by a space) without transforming it into a link. This commit ensures a space is added after the URL if it is pasted as text before a line break (introduced by a BR or a block element). Issue #3: pasting image/video URL + other text fragments The powerbox commands are not compatible with an image/video URL + text or multiple URLs. Choosing one of its commands will lead to loss of all text fragments but the last valid URL. This commit ensures the powerbox options are only offered when a single image/video URL is pasted. Issue #4: pasting image URL inside an exisiting link Even though the Editor's specs do not allow a URL to be transformed into a link when pasting it inside an exisiting link, pasting an image URL inside a link would open the powerbox with 3 commands, one of them allowing to transform the URL into a link. This commit ensures the "Paste as URL" command is not present in the powerbox in such case. When pasting an image URL inside an "isolated" link (after having clicked on it and making it the only contenteditable in the document), the cursor disappeared, preventing the powerbox to open (it would open later when insering text somewhere outside the link). This commit fixes it by restoring the cursor position after pasting an image URL inside an isolated link. Issue #5: selection restore after pasting and UNDO After pasting a valid URL (not image or video) on a non-collapsed selection followed by UNDO, the selection was not restored to the original range. This was due to not recording the selection in the current history step before creating the link. This commit introduces a call to _recordHistorySelection() before creating a link. After pasting text composed of a valid URL between text fragments on a non-collapsed selection followed by UNDO, the selection was again not properly restored. This time, because each text fragment insertion was done via execCommand, which calls _computeHistorySelection and thus overwrites the current step's selection. This commits calls _applyCommand instead, which skips the computing of new selections and keeps the one done onSelectionChange. Issue #6: unnecessary regex matching Before this commit, every text fragment in "text URL text" was being prepended with "https://" and a match with the URL regex was attempted. Every fragment (text or URL) was also probed for a match with the youtube video URL regex, but such matches would be discarded anyway in apps where the allowCommandVideo option is set to false. This commit avoids unnecessary searching with regular expressions by treating pasting of a single URL (possibly an image or video URL that would lead to powerbox commands) differently than URLs pasted among other text fragments or other URLs (see issue #2 above). Issue #7: historyPauseSteps with argument This function takes no arguments. Yet, it was called with the string 'onPaste' as argument. This commit removes it. task-3099012
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
This behavior is less surprising, less error-prone.
Unfortunately I don't know yet how to run the tests.
I've found some tests in
openerp/addons/test_new_apibut none which seems to test the previous behavior.