add action for audience-manager-cd #39

chiarabdy · 2020-04-30T16:02:28Z

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

I have signed the Adobe Open Source CLA.
My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.

codecov · 2020-04-30T16:04:34Z

Codecov Report

Merging #39 into master will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master       #39   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           17        18    +1     
  Lines          302       311    +9     
  Branches        39        39           
=========================================
+ Hits           302       311    +9

Impacted Files	Coverage Δ
generators/add-action/analytics/index.js	`100.00% <ø> (ø)`
generators/add-action/campaign-standard/index.js	`100.00% <ø> (ø)`
generators/add-action/customer-profile/index.js	`100.00% <ø> (ø)`
generators/add-action/generic/index.js	`100.00% <ø> (ø)`
generators/add-action/index.js	`100.00% <ø> (ø)`
generators/add-action/target/index.js	`100.00% <ø> (ø)`
lib/constants.js	`100.00% <ø> (ø)`
generators/add-action/audience-manager-cd/index.js	`100.00% <100.00%> (ø)`
... and 4 more

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 dd54ad3...3eea407. Read the comment docs.

…nto audienceManager-cd

moritzraho

@chiarabdy Thanks a lot for your PR, it's a nice one!
I've added a few comments regarding a change that has happened where the orgId should not be passed as a default param but is passed through the request header x-gw-ims-org-id.
A part from this and a few nitpicks, not much to say 😄

moritzraho · 2020-05-08T15:06:34Z

generators/add-action/audience-manager-cd/index.js

+    const audienceManagerClient = await AudienceManagerCD.init(params.orgId, params.apiKey, token)
+
+    // get Customer Profile from Audience Manager Customer Data API
+    const profiles = await audienceManagerClient.getProfile(


some formatting here:

await audienceManagerClient.getProfile({ dataSourceId: params.dataSourceId, id: params.id })

moritzraho · 2020-05-08T15:51:22Z

generators/add-action/audience-manager-cd/index.js

+    this.props = {
+      description: 'This is a sample action showcasing how to access the Adobe Audience Manager Customer Data API',
+      // eslint-disable-next-line quotes
+      requiredParams: `['apiKey', 'orgId']`,


Ok so two things regarding requiredParams:

you did good adding the requiredParams for default (.env and manifest) variables, but you should also add it for request parameters: so add dataSourceId and id

There is a new change where orgId is passed as a header through x-gw-ims-org-id, could you do the same change as in https://github.com/adobe/generator-aio-app/pull/44/files#diff-33c1a44418497d1f2fddbe457c86e7e3R24-R29 ? That also means that orgId is not a default variable in your case anymore (should not be added to manifest and .env)

awesome
according to the last changes of orgId from requiredparams to requiredheaders
i will have to change the stub-action.js as well to pass my tests, but in this case the other action templates tests will failed because of the existing orgId as a paramters too
i will push the changes after your pull request been merged

Hi @chiarabdy good point thanks for pointing this out, the PR just got merged now :)

moritzraho · 2020-05-08T15:51:41Z

generators/add-action/audience-manager-cd/index.js

+      dotenvStub: {
+        label: 'please provide your Adobe I/O Audience Manager Customer Data integration org id and api key',
+        vars: [
+          'AUDIENCE_MANAGER_CD_ORG_ID',


as mentioned above remove org id here

moritzraho · 2020-05-08T15:51:53Z

generators/add-action/audience-manager-cd/index.js

+        ]
+      },
+      actionManifestConfig: {
+        inputs: { LOG_LEVEL: 'debug', orgId: '$AUDIENCE_MANAGER_CD_ORG_ID', apiKey: '$AUDIENCE_MANAGER_CD_API_KEY' },


as mentioned above remove org id here

moritzraho · 2020-05-08T15:55:00Z

generators/add-action/audience-manager-cd/templates/getProfile.test.js

+    mockLoggerInstance.error.mockReset()
+  })
+
+  const fakeRequestParams = { orgId: 'fakeId', apiKey: 'fakeKey', __ow_headers: { authorization: 'Bearer fakeToken' } }


you will need to change the tests because the orgId is now passed via __ow_headers: { 'x-gw-ims-org-id' you can take a look at changes done here https://github.com/adobe/generator-aio-app/pull/44/files#diff-380b645bdf12ec16bd303d123d4cba0a it's probably similar for you

this will also need to change accordingly when adding the id and dataSourceId as requiredParams

test/generators/add-action/audience-manager-cd.test.js

…nto audienceManager-cd

moritzraho · 2020-05-11T15:09:01Z

generators/add-action/audience-manager-cd/templates/getProfile.test.js

+        }
+      })
+    })
+  })


newline missing? or bad indentation?

moritzraho · 2020-05-11T15:10:39Z

generators/add-action/audience-manager-cd/index.js

+        label: '## please provide your Adobe I/O Audience Manager Customer Data integration api key, id and dataSourceId',
+        vars: [
+          'AUDIENCE_MANAGER_API_KEY',
+          'AUDIENCE_MANAGER_ID',


Id and dataSourceId are request parameters, not default parameters you only need AUDIENCE_MANAGER_API_KEY

moritzraho · 2020-05-12T14:08:52Z

@chiarabdy lgtm !

add action for audience-manager-cd

9170f4f

chiarabdy requested a review from moritzraho April 30, 2020 16:03

meryllblanchet added the enhancement New feature or request label May 4, 2020

chiarabdy force-pushed the audienceManager-cd branch from 70ba75f to 9170f4f Compare May 4, 2020 12:28

Chiar added 2 commits May 5, 2020 09:48

Merge branch 'master' of https://github.com/adobe/generator-aio-app i…

0f688f4

…nto audienceManager-cd

remove am-cd package from stub-action.js

e918869

chiarabdy force-pushed the audienceManager-cd branch from eb4c498 to e918869 Compare May 5, 2020 09:03

moritzraho requested changes May 8, 2020

View reviewed changes

Chiar added 4 commits May 8, 2020 18:09

Merge branch 'master' of https://github.com/adobe/generator-aio-app i…

1f0e57e

…nto audienceManager-cd

Merge branch 'master' of https://github.com/adobe/generator-aio-app i…

3ca2b21

…nto audienceManager-cd

Merge branch 'master' of https://github.com/adobe/generator-aio-app i…

83aee22

…nto audienceManager-cd

PR-review - updates

ab0782d

chiarabdy force-pushed the audienceManager-cd branch from d90d630 to ab0782d Compare May 11, 2020 09:13

rename .env vars

d148d33

moritzraho approved these changes May 11, 2020

View reviewed changes

chiarabdy force-pushed the audienceManager-cd branch from 74792ce to d148d33 Compare May 11, 2020 16:27

Chiar added 2 commits May 11, 2020 18:27

change the default vars in .env and actionManifestConfig

53be09a

require id, dataSourceId in the header

3eea407

moritzraho merged commit 5e2c52b into master May 12, 2020

moritzraho deleted the audienceManager-cd branch May 12, 2020 14:09

gr00nd mentioned this pull request Feb 5, 2024

[Snyk] Upgrade @adobe/generator-app-common-lib from 0.3.3 to 2.0.2 Gr00nd-INC/generator-aio-app#3

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add action for audience-manager-cd #39

add action for audience-manager-cd #39

chiarabdy commented Apr 30, 2020 •

edited

Loading

codecov bot commented Apr 30, 2020 •

edited

Loading

moritzraho left a comment

moritzraho May 8, 2020

moritzraho May 8, 2020

chiarabdy May 11, 2020 •

edited

Loading

moritzraho May 11, 2020

moritzraho May 8, 2020

moritzraho May 8, 2020

moritzraho May 8, 2020

moritzraho May 8, 2020

moritzraho May 11, 2020

moritzraho May 11, 2020

moritzraho commented May 12, 2020

add action for audience-manager-cd #39

add action for audience-manager-cd #39

Conversation

chiarabdy commented Apr 30, 2020 • edited Loading

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

codecov bot commented Apr 30, 2020 • edited Loading

Codecov Report

moritzraho left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chiarabdy May 11, 2020 • edited Loading

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

moritzraho commented May 12, 2020

chiarabdy commented Apr 30, 2020 •

edited

Loading

codecov bot commented Apr 30, 2020 •

edited

Loading

chiarabdy May 11, 2020 •

edited

Loading