feat: implement auth attributes directly at the child level #344

dhandsar-aws · 2022-01-21T18:56:33Z

Issue #, if available:

Description of changes:

Moving auth attribute usage to child components to avoid creating top-level auth statement.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

dpilch · 2022-01-21T19:01:24Z

Branches seem out of sync. I think feature/auth-binding-in-child-components needs to be rebased on develop.

alharris-at · 2022-01-21T19:18:51Z

Branches seem out of sync. I think feature/auth-binding-in-child-components needs to be rebased on develop.

I'd rebase against the q1-release tagged release branch instead, since that's where I assume we'll be pushing these.

codecov-commenter · 2022-01-21T21:07:46Z

Codecov Report

Merging #344 (6ad0b73) into feature/auth-binding-in-child-components (5e77db9) will decrease coverage by 0.01%.
The diff coverage is 95.00%.

❗ Current head 6ad0b73 differs from pull request most recent head 91413b1. Consider uploading reports for the commit 91413b1 to get more accurate results

@@                             Coverage Diff                              @@
##           feature/auth-binding-in-child-components     #344      +/-   ##
============================================================================
- Coverage                                     93.81%   93.80%   -0.02%     
============================================================================
  Files                                            36       36              
  Lines                                          1277     1291      +14     
  Branches                                        287      290       +3     
============================================================================
+ Hits                                           1198     1211      +13     
- Misses                                           77       78       +1     
  Partials                                          2        2

Impacted Files	Coverage Δ
packages/codegen-ui/lib/types/studio-types.ts	`100.00% <ø> (ø)`
...egen-ui-react/lib/react-component-render-helper.ts	`90.24% <95.00%> (+0.15%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5e77db9...91413b1. Read the comment docs.

alharris-at

Overall looking good! Just a few minor comments.

alharris-at · 2022-01-21T21:47:48Z

packages/codegen-ui/lib/renderer-helper.ts

+  if (hasAuthProperty(component)) {
+    return true;
+  }
+  if (component.children && component.children.length > 0) {


Length check here is probably not required.

Good point, Removed

alharris-at · 2022-01-21T22:00:24Z

packages/codegen-ui-react/lib/react-component-render-helper.ts

+export function isAuthProperty(prop: ComponentPropertyValueTypes): prop is StudioComponentAuthProperty {
+  return 'userAttribute' in prop;
+}
+


Do you want to push this up to the renderHelper, since you basically reimplement this check in renderer-helper inside the hasAuth check.

Good suggestion, but more changes/refactor but I too feel its worth doing it now so its helpful in future :)

I think this needs to move to renderer-helper class, codegen-ui doesn't depend on codegen-ui-react, so I'm not sure how this'll build as-is.

alharris-at · 2022-01-21T22:02:48Z

packages/codegen-ui-react/lib/__tests__/studio-ui-json/componentWithUserSpecificAttributes.json

-          "bindingProperties": {
-            "property": "customUserAttributeIcecream"
-          }
+          "userAttribute": "customUserAttributeIcecream"


Could you leave this as custom:favorite_icecream (this is the actual format which a custom attribute takes from cognito.

dhandsar-aws · 2022-01-21T22:08:53Z

Branches seem out of sync. I think feature/auth-binding-in-child-components needs to be rebased on develop.

I'd rebase against the q1-release tagged release branch instead, since that's where I assume we'll be pushing these.

Done

* chore: upgrade to follow-redirects@1.14.7 (#337) resolves dependabot warning * feat: expose renderThemeJson method to generating themes in studio ui (#340) (#346) * feat: implement auth attributes directly at the child level (#344) Moved auth attribute usage to child components to avoid creating top-level auth statements. Co-authored-by: Dane Pilcher <dppilche@amazon.com> Co-authored-by: Al Harris <91494052+alharris-at@users.noreply.github.com>

dhandsar-aws requested a review from a team January 21, 2022 18:56

dhandsar-aws changed the title ~~feat: auth binding in child components~~ feat: implement auth attributes directly at the child level Jan 21, 2022

dpilch changed the base branch from feature/auth-binding-in-child-components to tagged-release/q1-release January 21, 2022 21:10

dhandsar-aws force-pushed the auth-binding-in-child-components branch from 91413b1 to 608db54 Compare January 21, 2022 21:20

dpilch approved these changes Jan 21, 2022

View reviewed changes

alharris-at reviewed Jan 21, 2022

View reviewed changes

alharris-at changed the base branch from tagged-release/q1-release to feature/auth-binding-in-child-components January 21, 2022 22:53

dhandsar-aws force-pushed the auth-binding-in-child-components branch 2 times, most recently from 67ba67a to 922d24d Compare January 24, 2022 20:18

feat: implement auth attributes directly at the child level

8485844

dhandsar-aws force-pushed the auth-binding-in-child-components branch from 922d24d to 8485844 Compare January 24, 2022 20:25

dhandsar-aws merged commit 7fdf974 into feature/auth-binding-in-child-components Jan 24, 2022

dhandsar-aws deleted the auth-binding-in-child-components branch January 24, 2022 20:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: implement auth attributes directly at the child level #344

feat: implement auth attributes directly at the child level #344

dhandsar-aws commented Jan 21, 2022

dpilch commented Jan 21, 2022

alharris-at commented Jan 21, 2022

codecov-commenter commented Jan 21, 2022

alharris-at left a comment

alharris-at Jan 21, 2022

dhandsar-aws Jan 21, 2022

alharris-at Jan 21, 2022

dhandsar-aws Jan 21, 2022

dhandsar-aws Jan 21, 2022

alharris-at Jan 21, 2022

alharris-at Jan 21, 2022

dhandsar-aws Jan 21, 2022

dhandsar-aws commented Jan 21, 2022

feat: implement auth attributes directly at the child level #344

feat: implement auth attributes directly at the child level #344

Conversation

dhandsar-aws commented Jan 21, 2022

dpilch commented Jan 21, 2022

alharris-at commented Jan 21, 2022

codecov-commenter commented Jan 21, 2022

Codecov Report

alharris-at 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

dhandsar-aws commented Jan 21, 2022