Enabling entity level setting for `sourceCapture` optional settings #1326

travjenkins · 2024-10-24T21:10:34Z

Issues

#1135

Changes

1135

Wire up new options for delta updates and schema naming
Add new optional settings for the add dialoge
Ability to add custom secondary button to the entity selector modal

Misc

Update how we add annotations to the ajv instance
Just directly store the default ajv instance and stop using iife
Renaming variables that are expecting components
Moved some stuff related to resource config to a shared handler

Tests

Manually tested

scenarios you manually tested

Automated tests

unit testing covered

Playwright tests ran locally

Screenshots

Long name / wide view

Long name / narrow view

…ty-level

…ncompatible-schema-entity-level

for sure - but want the UI stubbed out

…ty-level

Starting to move delta updates into a stand alone component

…ncompatible-schema-entity-level

moving details into a single chip

Adding some padding around the top and bottom

Getting tests working

…operly) Starting to stub out where the WASM calls will go Adding a Nullable typescript util

Minor renaming More work on handling name vs setting changes

config so they are not going to be like backfill Getting the cancel button working right

Starting to check the props in the forms

…ncompatible-schema-entity-level

Moving options into stand alone file Cleaning up unused values

travjenkins · 2024-12-02T16:38:40Z

src/components/shared/AutoComplete/DefaultProps.tsx

This made sharing stuff for auto complete harder so fixing.

travjenkins · 2024-12-02T16:48:40Z

src/types/jsonforms.ts

Added the documentation comments here now that we do not need to declare these multiple times

…ncompatible-schema-entity-level

kiahna-tucker

Releasing review comments while testing is underway.

kiahna-tucker · 2024-12-03T22:50:25Z

src/lang/en-US/Workflows.ts

+
+    // Schema Mode
+    'schemaMode.header': `Source Capture Schema Mode`,
+    'schemaMode.message': `How should the schema of the materialization binding be set.`,


This sentence is phrased like a question. I would prefer for it to be declarative in nature: How the schema of the materialization should be set. That said, this piece of content reads a tad awkwardly out of context.

kiahna-tucker · 2024-12-03T22:52:09Z

src/lang/en-US/Workflows.ts

+    'schemaMode.message': `How should the schema of the materialization binding be set.`,
+    'schemaMode.input.label': `Set new bindings schemas as`,
+
+    'schemaMode.error.message': `The current setting "{currentSetting}" does not match a known option. Please update or remove.`,


This may become a common error message for menus of this nature. We should keep an eye on how often this is repeated. The onIncompatibleSchemaChange fields employ a similar error message.

kiahna-tucker · 2024-12-03T22:54:00Z

src/lang/en-US/Workflows.ts

+    'schemaMode.options.leaveEmpty.description': `Leave the materialization binding's schema field empty, therefore falling back to the default schema of the materialization.`,
+
+    'schemaMode.options.fromSourceName.label': `From Source Name`,
+    'schemaMode.options.fromSourceName.description': `Use the 2nd-to-last component of the collection name as the schema of the materialization binding.`,


Do you know how 2nd-to-last will be translated?

Depends on the language and if they have the concept of numerical ordering.

kiahna-tucker · 2024-12-03T22:55:52Z

src/services/jsonforms/index.ts

-    return Object.hasOwn(schema, Annotations.oAuthProvider);
-};
+const isOAuthConfig = (schema: JsonSchema): boolean =>
+    Object.hasOwn(schema, Annotations.oAuthProvider);


Calling out the presence of Object.hasOwn() in the event you would like to replace it.

Yeah - I think we might just end up polyfilling it possibly. Like make it the only one off. Cause it'll be easier.

src/stores/Binding/Store.ts

kiahna-tucker · 2024-12-03T23:00:08Z

src/stores/Binding/types.ts

+    // Control sourceCapture optional settings
+    sourceCaptureTargetSchemaSupported: boolean;
+    sourceCaptureDeltaUpdatesSupported: boolean;


Why were these pieces of state added to the bindings store? If you explained this before the holiday, would you mind jogging my memory?

They are settings based on the resource config. Eventually we'll probably be adding support to mass change these settings within the bindings as well.

kiahna-tucker · 2024-12-03T23:04:22Z

src/utils/entity-utils.ts

+    if (sourceCapture) {
+        draftSpec.sourceCapture = sourceCapture;
+    } else {
+        delete draftSpec.sourceCapture;


Out of curiosity, do you prefer delete to Lodash's omit?

kiahna-tucker · 2024-12-03T23:06:01Z

src/components/materialization/SourceCapture/CancelSourceCaptureButton.tsx

+    });
+
+    return (
+        <Button variant="contained" onClick={close}>


Why use the contained variant for this Cancel button and not one of lesser weight?

Copy and paste error - it is supposed to be outlined like it is today.

kiahna-tucker

Potential Defects

Enter the edit workflow for a materialization without a linked capture, click the Source from Capture CTA, select a capture table row, click the Default schema from source name switch so it is in the on position, click the Continue button, and observe that the summary chip displays an X icon next to Schema Names. This happens regardless of which switch is turn on; the summary chip does not consistently reflect the state of either (or both) settings when turned on in the aforementioned scenario. It should be noted that the summary chip does briefly show the correct state while the form is active, but is reset to the default state when the form returns to an idle state.
Enter the edit workflow for a materialization with a linked capture that has one, sourceCapture setting set, click the Source from Capture CTA, select a different capture table row, and observe that the previously linked capture row is shadow selected (i.e., greyed-out with its checkbox checked).

Additional Comments

It does feel a little strange for the backend to cleanup sourceCapture properties with a default value. Presently, there is no benefit to mirroring the backend behavior here; merely wanted to share my reaction for the record.

References

Potential bug 2 | Shadow selected table row

pass the same props

to build up the settings and used the latest settings and ignore if the source capture is not set

travjenkins · 2024-12-05T02:02:05Z

Pushed fixes - ready for another review ad testing

…ty-level

kiahna-tucker

Approved with minimal testing of the latest round of changes.

travjenkins added 7 commits October 24, 2024 13:45

Moving file and adding new section

25f61b0

Merge branch 'main' into travjenkins/feature/incompatible-schema-enti…

d5083a7

…ty-level

Merge remote-tracking branch 'origin/main' into travjenkins/feature/i…

72d8b19

…ncompatible-schema-entity-level

Starting wiring up stuff. Going to end up sharing a lot of this code

fea1924

for sure - but want the UI stubbed out

Merge branch 'main' into travjenkins/feature/incompatible-schema-enti…

8075993

…ty-level

Switching to new design where both settings are just switches

faea0de

Starting to move delta updates into a stand alone component

Merge remote-tracking branch 'origin/main' into travjenkins/feature/i…

2be4a10

…ncompatible-schema-entity-level

travjenkins changed the title ~~Add entity level binding options~~ Add binding property prefill ability to collection selection Nov 12, 2024

travjenkins added 22 commits November 12, 2024 17:33

adding items to the store

3be6125

moving details into a single chip

Moving everything inside the chip

c7b33d1

Moving everything inside the chip as requested

688e424

Fixing the delete button size

4dca48b

Adding some padding around the top and bottom

Adding some more space

7777ede

more spacing tweaks

3349e4a

tweaking spacing a bit more

49055ca

final spacing tweak

d8af346

more work to get the styling right

f45097f

this is so gross but works and I am out of patience

a813a3d

Starting to wire up passing around the new props

e68e51c

Getting tests working

Making this fetch just grab the object we need

3dc7ff6

TODO

8c921a0

moving around and adding util functions

ecc9ac6

Doing deeper compares to know when to update (so flags are updated pr…

752b262

…operly) Starting to stub out where the WASM calls will go Adding a Nullable typescript util

Getting some more specific typing handled

003bda8

Minor renaming More work on handling name vs setting changes

Won't need binding index because these settings are INSIDE the resource

e661964

config so they are not going to be like backfill Getting the cancel button working right

reducing duplication for json schema annotations

b7e583f

Starting to wire up if the optional settings are supported

954b42e

Moving some shared stuff out to make file smaller

d6f7828

Moving some content to lang files

eaf7f57

Starting to check the props in the forms

Merge remote-tracking branch 'origin/main' into travjenkins/feature/i…

9635b2d

…ncompatible-schema-entity-level

travjenkins added 8 commits December 2, 2024 11:02

Removing an unused file from a previous approach

4471f04

cleaning up logging

c062aa4

Not used

c449b99

pr: noting why a hack exists

76683b3

removing TODO as this seems alright now

a52b7ea

Using the shared hook and merging the updates in

a0fb0c7

Renaming files

1fdd888

Moving options into stand alone file Cleaning up unused values

not needed

8349a56

travjenkins commented Dec 2, 2024

View reviewed changes

src/components/shared/AutoComplete/DefaultProps.tsx Outdated

Copy link

Member Author

travjenkins Dec 2, 2024

Choose a reason for hiding this comment

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

This made sharing stuff for auto complete harder so fixing.

travjenkins added 3 commits December 2, 2024 11:40

marking todo

c96ca7a

marking todo

ff5d994

removing commented code

4eb0b59

travjenkins commented Dec 2, 2024

View reviewed changes

travjenkins added 2 commits December 2, 2024 11:52

Not needed

b028f0b

Merge remote-tracking branch 'origin/main' into travjenkins/feature/i…

d44619e

…ncompatible-schema-entity-level

kiahna-tucker reviewed Dec 3, 2024

View reviewed changes

kiahna-tucker added the change:planned This is a planned change label Dec 3, 2024

kiahna-tucker requested changes Dec 3, 2024

View reviewed changes

travjenkins added 6 commits December 4, 2024 09:51

PR: copy and paste design bug

a93eedb

Need to always make sure the input assume it is a boolean

0e3dfaf

Need the memo to stop extra renders

5235df5

keeping the optional settings rendering like buttons so we can always

b9f4c74

pass the same props

This needs to get each independent part of the def because it needs

be7d94d

to build up the settings and used the latest settings and ignore if the source capture is not set

Similar to add where it is safest to piece this together

a7879c7

travjenkins requested a review from kiahna-tucker December 5, 2024 02:01

Merge branch 'main' into travjenkins/feature/incompatible-schema-enti…

6d51ce4

…ty-level

kiahna-tucker approved these changes Dec 5, 2024

View reviewed changes

travjenkins merged commit 96749d6 into main Dec 5, 2024
3 checks passed

travjenkins deleted the travjenkins/feature/incompatible-schema-entity-level branch December 5, 2024 19:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enabling entity level setting for `sourceCapture` optional settings #1326

Enabling entity level setting for `sourceCapture` optional settings #1326

travjenkins commented Oct 24, 2024 •

edited

Loading

travjenkins Dec 2, 2024

travjenkins Dec 2, 2024

kiahna-tucker left a comment

kiahna-tucker Dec 3, 2024

kiahna-tucker Dec 3, 2024

kiahna-tucker Dec 3, 2024

travjenkins Dec 4, 2024

kiahna-tucker Dec 3, 2024

travjenkins Dec 4, 2024

kiahna-tucker Dec 3, 2024

travjenkins Dec 4, 2024

kiahna-tucker Dec 3, 2024

travjenkins Dec 4, 2024

kiahna-tucker Dec 3, 2024

travjenkins Dec 4, 2024

kiahna-tucker left a comment •

edited

Loading

travjenkins commented Dec 5, 2024

kiahna-tucker left a comment

Enabling entity level setting for sourceCapture optional settings #1326

Enabling entity level setting for sourceCapture optional settings #1326

Conversation

travjenkins commented Oct 24, 2024 • edited Loading

Issues

Changes

1135

Misc

Tests

Manually tested

Automated tests

Playwright tests ran locally

Screenshots

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kiahna-tucker left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kiahna-tucker left a comment • edited Loading

Choose a reason for hiding this comment

Potential Defects

Additional Comments

References

travjenkins commented Dec 5, 2024

kiahna-tucker left a comment

Choose a reason for hiding this comment

Enabling entity level setting for `sourceCapture` optional settings #1326

Enabling entity level setting for `sourceCapture` optional settings #1326

travjenkins commented Oct 24, 2024 •

edited

Loading

kiahna-tucker left a comment •

edited

Loading