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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Differential (Additive/Removal) patching when switching between design / debug #3124

Conversation

apisaint
Copy link
Contributor

@apisaint apisaint commented Feb 26, 2021

馃槱 Finally got tired of dealing with it so I tried fixing it.

This patch adds differential patching for imports that occur during the switching between design and debug tabs inside of Designer. As reported through #2971, and others, this patch favors existing data over imported data, except urls (see below), values that exist already on the document will remain unchanged, only new values (including array based values) will be added / removed.

This also includes a bypass feature for urls, currently these options are not exposed through the interface but could be. This feature is an object to allow for future properties for preference-based patching.

This also allows endpoints to stay in position when moved or grouped in folders.

馃憞 Included:

  • Adds options.enableDiffBasedPatching and options.enableDiffDeep to importRaw for backwards compat.
  • Adds options.bypassDiffProps.url for url bypassing and use an object for future items.
  • Adds diffPatchObj based on differential patching for objects (works with arrays as well).

馃挱 Future ideas:

  • hasBeenModifiedByUser property map object to allow changing properties that haven't been touched by the user with options.
  • Notice icon when clicked displays dialogue with changes that a user can choose to apply. Underlying diff method can be augmented to create such patch objects or use a lib.

鈿狅笍 Notes:

Basically this makes changes additive, which is better than the current scenario especially when paired with #2979, what does that imply? See below.

  • If it exists it won鈥檛 change unless a bypass option is created.
  • When the url changes for a Request, when the bypass is enabled (true by default here) it will update the url in the url bar but it does not change the name, so the sidebar will not auto-update. This can be resolved in a future or with a modified check for the name using the future idea explained above.
  • When items / values inside of the Headers change it does not auto-update, this would require either a bypass option, or rely on the future idea.

Related #3038
Closes #2971, #2882, #2442

@apisaint apisaint force-pushed the fix/object-patching-on-activity-switch branch from 2b64303 to b24cb3d Compare February 26, 2021 11:57
@reynolek
Copy link
Contributor

We have this on our list of issues to tackle, thanks for taking the time to look into it! Going to add @nijikokun to review it from a product perspective along with @develohpanda and @dimitropoulos for code.

@nijikokun
Copy link
Contributor

Oh wow. Taking a look now 馃憖

  • backwards compat.

Beautiful 馃檶

This also allows endpoints to stay in position when moved or grouped in folders.

Absolutely amazing 馃帀

  • it will update the url in the url bar but it does not change the name, so the sidebar will not auto-update. This can be resolved in a future or with a modified check for the name using the future idea explained above.

While we do have modified you're right we don't have something that tells us whether it's been modified by the user or through creation, at least to my knowledge, so I agree this can be a future improvement. 馃憤

  • Notice icon when clicked displays dialogue with changes that a user can choose to apply. Underlying diff method can be augmented to create such patch objects or use a lib.

Love this idea 馃挴, maybe this could also be expanded to sync at some point 馃

  • hasBeenModifiedByUser property map object to allow changing properties that haven't been touched by the user with options.

Makes sense, track properties that change, and update the ones that haven't, seems like a good future improvement. 馃挴

  • Adds options.bypassDiffProps.url for url bypassing and use an object for future items.

馃憤


Amazing, love the breakdown. Found an issue in the code, but other than that it's definitely an improvement over the current situation and I'll take it.

@DMarby DMarby changed the base branch from develop to release/2021.1 March 3, 2021 12:35
@dimitropoulos dimitropoulos force-pushed the fix/object-patching-on-activity-switch branch from 99f64a6 to 8d94f2e Compare March 3, 2021 16:43
@dimitropoulos
Copy link
Contributor

(working on making sure I correctly resolve the merge conflict of the prior commit on master then this is next up to be merged)

@dimitropoulos dimitropoulos force-pushed the fix/object-patching-on-activity-switch branch from 8d94f2e to 3abe21f Compare March 3, 2021 17:14
This patch adds differential patching for imports that occur during the switching between
design and debug tabs inside of Designer. As reported through Kong#2971, and others, this patch
favors existing data over imported data, values that exist already on the document will remain
unchanged, only new values (including array based values) will be added / removed.

This also includes a bypass feature for urls, currently these options are not exposed through
the interface but could be. This feature is an object to allow for future properties for
preference-based patching.

- Adds `options.enableDiffBasedPatching` and `options.enableDiffDeep` to `importRaw` for backwards compat.
- Adds `options.bypassDiffProps.url` for url bypassing and use an object for future items.
- Adds `diffPatchObj` based on differential patching for objects (works with arrays as well).

Future ideas:

- `hasBeenModifiedByUser` property map object to allow changing properties that haven't been touched by the user with options.

fixes: Kong#2971, Kong#2882, Kong#3038, Kong#2442
- also adds jest (which was (mistakenly) not there before)
- does not call `.hasOwnProperty` directly, per https://eslint.org/docs/rules/no-prototype-builtins (which, we will add more formally at a later date)
this ensures the behavior of the initial PR is more preserved
@dimitropoulos dimitropoulos force-pushed the fix/object-patching-on-activity-switch branch from 0b13f9a to 820c296 Compare March 3, 2021 17:32
@dimitropoulos dimitropoulos merged commit a2bc704 into Kong:release/2021.1 Mar 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Updated requests rolled back to default after switching to DESIGN mode and then back to DEBUG mode
4 participants