[ADD] New module: web_readonly_bypass #162

JonathanNEMRY · 2015-07-02T08:54:17Z

This Module provides a solution to the problem of the interaction between 'readonly' attribute and 'on_change' attribute when used together

… readonly fields

adrienpeiffer · 2015-07-02T09:24:04Z

web_readonly_bypass/README.rst

+------------
+
+* Jonathan Nemry <jonathan.nemry@acsone.eu>
+* Laetitia Gangloff <laetitia.gangloff.eu>


I think email address isn't correct

adrienpeiffer · 2015-07-02T09:27:34Z

adrienpeiffer · 2015-07-02T09:32:06Z

Little remarks on readme file and about header. Otherwise, 👍

pedrobaeza · 2015-07-02T11:37:47Z

web_readonly_bypass/__openerp__.py

+#
+#    Authors: Nemry Jonathan & Laetitia Gangloff
+#    Copyright (c) 2014 Acsone SA/NV (http://www.acsone.eu)
+#    All Rights Reserved


Remove this useless line

pedrobaeza · 2015-07-02T11:40:00Z

Very interesting feature! 👍 (code review)

petrus-v · 2015-07-02T20:41:57Z

web_readonly_bypass/views/readonly_bypass.xml

+    <data>
+        <template id="assets_backend" name="readonly_bypass" inherit_id="web.assets_backend">
+            <xpath expr="." position="inside">
+                <script type="text/javascript" src="/readonly_bypass/static/src/js/readonly_bypass.js"></script>


As you rename module you miss to change the path:

Should be

<script type="text/javascript" src="/web_readonly_bypass/static/src/js/readonly_bypass.js"></script>

* Use a better test condition * Fix the email address [FIX] * rename the path and web instance just as the module was

petrus-v · 2015-07-03T07:32:43Z

Excellent 👍

pedrobaeza · 2015-07-09T07:31:24Z

DO NOT MERGE YET!

Thinking about this carefully, this module inverts the behaviour by default, so it applies to all existing codebase, and can lead to unexpected behaviours in modules that rely on the volatibility of onchange to change data, but they don't want to store it.

The module must act the contrary: use the normal behaviour by default, and include a context key not_filter_out_readonly or similar that allows that someone include this dependency and this value in the field context when he wants to change the behaviour.

sbidoul · 2015-07-09T12:44:38Z

Although I don't have any concrete cases in mind, I agree with @pedrobaeza: it's risky to have a module that changes the standard behavior system wide, as this may have unexpected side effects.

@petrus-v since you are testing it these days, would you mind doing that change?

petrus-v · 2015-07-09T13:06:00Z

@sbidoul

I agree with @pedrobaeza! This behavior will be safer for me as well, I'll make change on #164 as well!

I wonder how to set the context correctly, I expected to set it on the field or the entire view but it looks more complicated as I expected to forward it properly!

Thanks to take me in consideration!

pedrobaeza · 2015-07-09T14:18:44Z

I think the best approach is to define the context at field level, so you can fine-grain the control over each readonly field.

JonathanNEMRY · 2015-07-09T14:26:57Z

@petrus-v
an idea could be to say: if we want this behavior then add it into the context action of the form. And then the added context should simply be like that {'readonly_by_pass': True} or in a more complex way {'readonly_by_pass': ['concerned_field', '...', ...]}
By this way, it will be possible to extend the scope of this behavior, from a specific field to an entire form and the changes into the existing code are minimals

pedrobaeza · 2015-07-09T14:38:14Z

Well, I prefer the duality of the key in the action window context or the key in the field context.

petrus-v · 2015-07-09T14:44:53Z

web_readonly_bypass/static/src/js/readonly_bypass.js

+    function ignore_readonly(data, options, mode, context){
+        if (options){
+            if ('readonly_fields' in options && options['readonly_fields'] &&
+                    !('filter_out_readonly' in context && context['filter_out_readonly'] == true )) {


@JonathanNEMRY , @pedrobaeza

To be accurate, my problem was that, when I quickly tested to define context on the field like this on the view:

<field name="my_readonly_field" context="{'filter_out_readonly': True}" />

or on the action windows.

The javascript context parameter does'nt contain the filter_out_readonly properties :-/

It needs more investigations ;-)

JonathanNEMRY · 2015-07-09T15:17:12Z

Added a specific context into a field will be not propagated until the dataset or datasetbuffer action (create/write in our case). Context is propagated for relational fields if a context is present into the node:
build_context: function() { // only use the model's context if there is not context on the node var v_context = this.node.attrs.context;
This method is not called for other fields so I agree with @pedrobaeza to have a dual context key action or field but it will be more difficult and that's why I suggested to use the context action

pedrobaeza · 2015-07-09T15:19:31Z

Ok, I don't know about the difficulty in JS side, so go ahead with the one you propose, @JonathanNEMRY

JonathanNEMRY · 2015-07-10T11:54:29Z

So @petrus-v do you think it is OK for you to consider the special context key will be added into the action's context (ir.action.act_window)? (so it will be available into the self.context of create/write from the dataset classe )

petrus-v · 2015-07-10T12:57:55Z

@JonathanNEMRY

Yes I'm fine with that! I'll do the same. thanks for the tip!

petrus-v · 2015-08-06T16:14:54Z

@JonathanNEMRY I've done some works there https://github.com/petrus-v/web/tree/8.0-web_readonly_bypass but I'm going to be off for a month, I could create a PR on acsone repo first, then let you follow this PR, or I create a new fresh PR on OCA/web repo, or you can directly merge my branch (becarefull I rebase on the top of oca/8.0)? What do you prefer ?

JonathanNEMRY · 2015-08-07T09:46:02Z

@petrus-v you can make a PR on https://github.com/acsone/web/tree/8.0-web_readonly_bypass-jne and then I will take care of the next steps! see you next month ;)

petrus-v · 2015-08-07T09:52:39Z

@JonathanNEMRY

Ok, thanks, I'll do that.

readonly_by_pass. This context can either be a bool or field name array

BufferedDataSet Class

pedrobaeza · 2015-09-09T15:19:05Z

Any update on this?

8.0 web readonly bypass jne

petrus-v · 2015-09-28T15:22:50Z

@pedrobaeza, @sbidoul

I think we could replace work in progress tag by needs review tag as PR #164 was merged.

Obviously I give my 👍 as I've done the end of works, but I'm not sure it has values!

lmignon · 2015-09-28T18:55:30Z

web_readonly_bypass/README.rst

@@ -0,0 +1,86 @@
+.. image:: https://img.shields.io/badge/licence-AGPL--3-blue.svg
+    :alt: License: AGPL-3


Can you take a look to the latest version of the sample README.rst (https://raw.githubusercontent.com/OCA/maintainer-tools/master/template/module/README.rst) and fix the following issues?

add target to the image licence badge

put a lot of '===' above the module title to make the title a real title

add a link 'Try me on Runbot' in the usage section

* Improve .rst * user_strict into js * review Authors

JonathanNEMRY · 2015-10-01T16:22:36Z

Hi,
I just add some improvements referenced to the last review and I've also tested the module with @petrus-v 's changes. It seems OK for me now. Just some more thumbs up before merging.

lmignon · 2015-10-01T17:11:59Z

👍

sbidoul · 2015-10-02T07:05:08Z

web_readonly_bypass/README.rst

+------------
+
+* Jonathan Nemry <jonathan.nemry@acsone.eu>
+* Laetitia Gangloff <laetitia.gangloff@acsone.eu>


Add @petrus-v in contributors.

JonathanNEMRY · 2015-10-02T07:23:13Z

@petrus-v is added to the contributors into README.rst: sorry for this omission and thanks again for your contribution!

petrus-v · 2015-10-02T07:58:29Z

@JonathanNEMRY,

oh yes I missed it too, thanks! That was with pleasure ;), you did the hardest way

pedrobaeza · 2015-10-04T02:18:48Z

👍

[ADD] New module: web_readonly_bypass

Update all submodules + project + fix build

…constraint BSMTS-286 Remove BVR/ESR unique constraint

[ADD] web_readonly_bypass: allow to save an onchange modifications on…

749b47e

… readonly fields

This was referenced Jul 2, 2015

Porting readonly_bypass on v7 acsone/acsone-addons#69

Closed

remove readonly_bypass from acsone-addons when web_readonly_bypass is merged in OCA/web acsone/acsone-addons#70

Closed

JonathanNEMRY force-pushed the 8.0-web_readonly_bypass-jne branch from 8939d07 to 5c0c721 Compare July 2, 2015 09:21

adrienpeiffer reviewed Jul 2, 2015
View reviewed changes

legalsylvain added the needs review label Jul 2, 2015

legalsylvain added this to the 8.0 milestone Jul 2, 2015

pedrobaeza reviewed Jul 2, 2015
View reviewed changes

[IMP] web_readonly_bypass: port to OCA from acsone-addons

3260cd2

sbidoul force-pushed the 8.0-web_readonly_bypass-jne branch from 5c0c721 to 3260cd2 Compare July 2, 2015 12:37

petrus-v reviewed Jul 2, 2015
View reviewed changes

petrus-v mentioned this pull request Jul 2, 2015

7.0 web readonly bypass #164

Merged

[CHG] some code review:

3b98431

* Use a better test condition * Fix the email address [FIX] * rename the path and web instance just as the module was

petrus-v reviewed Jul 9, 2015
View reviewed changes

Pierre Verkest added 3 commits August 7, 2015 12:15

cherry-pick 790a853 by pass readonly fields only if context is set with

c4bc379

readonly_by_pass. This context can either be a bool or field name array

cherry-pick c639533 combine context, Get windows_action context in

56da0db

BufferedDataSet Class

fix readme as on v7

e96b66c

petrus-v mentioned this pull request Aug 7, 2015

8.0 web readonly bypass jne acsone/web#1

Merged

Merge pull request #1 from petrus-v/8.0-web_readonly_bypass-jne

2c53d07

8.0 web readonly bypass jne

sbidoul added needs review and removed work in progress labels Sep 28, 2015

lmignon reviewed Sep 28, 2015
View reviewed changes

[CHG] Code review

2140046

* Improve .rst * user_strict into js * review Authors

sbidoul reviewed Oct 2, 2015
View reviewed changes

[CHG] Improve contributors into README.rst

980c157

pedrobaeza added a commit that referenced this pull request Oct 4, 2015

Merge pull request #162 from acsone/8.0-web_readonly_bypass-jne

da4402d

[ADD] New module: web_readonly_bypass

pedrobaeza merged commit da4402d into OCA:8.0 Oct 4, 2015

yvaucher removed the needs review label Oct 4, 2015

sbidoul deleted the 8.0-web_readonly_bypass-jne branch October 22, 2016 15:01

leemannd pushed a commit to camptocamp/web that referenced this pull request May 29, 2018

Merge pull request OCA#162 from jcoux/update_all_submodules

94e37be

Update all submodules + project + fix build

vrenaville pushed a commit to camptocamp/web that referenced this pull request Jul 19, 2018

Merge pull request OCA#162 from p-tombez/bsmts-286_remove_esr_unique_…

cb9ff32

…constraint BSMTS-286 Remove BVR/ESR unique constraint

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ADD] New module: web_readonly_bypass #162

[ADD] New module: web_readonly_bypass #162

JonathanNEMRY commented Jul 2, 2015

adrienpeiffer Jul 2, 2015

adrienpeiffer commented Jul 2, 2015

adrienpeiffer commented Jul 2, 2015

pedrobaeza Jul 2, 2015

pedrobaeza commented Jul 2, 2015

petrus-v Jul 2, 2015

petrus-v commented Jul 3, 2015

pedrobaeza commented Jul 9, 2015

sbidoul commented Jul 9, 2015

petrus-v commented Jul 9, 2015

pedrobaeza commented Jul 9, 2015

JonathanNEMRY commented Jul 9, 2015

pedrobaeza commented Jul 9, 2015

petrus-v Jul 9, 2015

JonathanNEMRY commented Jul 9, 2015

pedrobaeza commented Jul 9, 2015

JonathanNEMRY commented Jul 10, 2015

petrus-v commented Jul 10, 2015

petrus-v commented Aug 6, 2015

JonathanNEMRY commented Aug 7, 2015

petrus-v commented Aug 7, 2015

pedrobaeza commented Sep 9, 2015

petrus-v commented Sep 28, 2015

lmignon Sep 28, 2015

JonathanNEMRY commented Oct 1, 2015

lmignon commented Oct 1, 2015

sbidoul Oct 2, 2015

JonathanNEMRY commented Oct 2, 2015

petrus-v commented Oct 2, 2015

pedrobaeza commented Oct 4, 2015

		@@ -0,0 +1,86 @@
		.. image:: https://img.shields.io/badge/licence-AGPL--3-blue.svg
		:alt: License: AGPL-3

[ADD] New module: web_readonly_bypass #162

[ADD] New module: web_readonly_bypass #162

Conversation

JonathanNEMRY commented Jul 2, 2015

adrienpeiffer Jul 2, 2015

Choose a reason for hiding this comment

adrienpeiffer commented Jul 2, 2015

adrienpeiffer commented Jul 2, 2015

pedrobaeza Jul 2, 2015

Choose a reason for hiding this comment

pedrobaeza commented Jul 2, 2015

petrus-v Jul 2, 2015

Choose a reason for hiding this comment

petrus-v commented Jul 3, 2015

pedrobaeza commented Jul 9, 2015

sbidoul commented Jul 9, 2015

petrus-v commented Jul 9, 2015

pedrobaeza commented Jul 9, 2015

JonathanNEMRY commented Jul 9, 2015

pedrobaeza commented Jul 9, 2015

petrus-v Jul 9, 2015

Choose a reason for hiding this comment

JonathanNEMRY commented Jul 9, 2015

pedrobaeza commented Jul 9, 2015

JonathanNEMRY commented Jul 10, 2015

petrus-v commented Jul 10, 2015

petrus-v commented Aug 6, 2015

JonathanNEMRY commented Aug 7, 2015

petrus-v commented Aug 7, 2015

pedrobaeza commented Sep 9, 2015

petrus-v commented Sep 28, 2015

lmignon Sep 28, 2015

Choose a reason for hiding this comment

JonathanNEMRY commented Oct 1, 2015

lmignon commented Oct 1, 2015

sbidoul Oct 2, 2015

Choose a reason for hiding this comment

JonathanNEMRY commented Oct 2, 2015

petrus-v commented Oct 2, 2015

pedrobaeza commented Oct 4, 2015