Skip to content

Airship Actions Destination #4946

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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jul 5, 2023
Merged

Airship Actions Destination #4946

merged 7 commits into from
Jul 5, 2023

Conversation

stayseesong
Copy link
Contributor

@stayseesong stayseesong commented Jun 26, 2023

Proposed changes

Merge timing

Once ready to go and stakeholders addressed the comments

Related issues (optional)

@stayseesong stayseesong requested a review from a team as a code owner June 26, 2023 21:37
@stayseesong stayseesong requested review from rchinn1 and removed request for a team June 26, 2023 21:37
@stayseesong stayseesong added the new-integration Added a new source or destination label Jun 26, 2023
@stayseesong stayseesong mentioned this pull request Jun 26, 2023
6. Click **Create Destination**.
7. Enter your Access Token and App Key. You can get your access token and app key by going to your Airship project and navigating to **Settings > Partner Integrations** and selecting Segment. Following the instructions there will create a Tag Group, Attributes, and provide the Access Token and App Key.
8. Select the appropriate data center.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jumpingGrendel this is the comment that @joe-ayoub-segment left from the previous PR (#4921). I'm copying it over to this PR so that you can address them here:

We could add some lines to explain which Preset Mappings are available, etc.

I think we should add a new heading, and explain how identifiers work. It's important for the customer to know that Segment won't create a user profile in Airship. i.e the "Airship Named User ID" field is required, and the value needs to be for a pre-existing Named User in Airship. Segment events cannot be used to create a user profile in Airship... etc...

Copy link
Contributor

Choose a reason for hiding this comment

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

@stayseesong and @joe-ayoub-segment have a look I added some deeper explanations for some of the behavior/concepts. thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jumpingGrendel where did you add the edits?

Copy link
Contributor

Choose a reason for hiding this comment

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

hmm @stayseesong when you closed the PR and reopened it did it lose the connection with my fork? urbanairship@8a4e417

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jumpingGrendel I didn't reopen the PR that you created. I had to close that PR out because the docs team was unable to edit it, so I had to create a new PR where the team can edit it

Copy link
Contributor

Choose a reason for hiding this comment

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

@stayseesong sorry I was unclear! The old PR wanted to merge my fork/branch into segmentio develop whereas for this new PR you've created a new branch in segmentio. I tried making my changes to your branch but I don't have permission.

john @ cantankerous ~/projects/segment-docs (PINT-1842)
└─ $ ▶ git push origin PINT-1842
Enumerating objects: 15, done.
Counting objects: 100% (15/15), done.
Delta compression using up to 8 threads
Compressing objects: 100% (7/7), done.
Writing objects: 100% (8/8), 648 bytes | 648.00 KiB/s, done.
Total 8 (delta 6), reused 0 (delta 0), pack-reused 0
remote: Resolving deltas: 100% (6/6), completed with 6 local objects.
remote: Bypassed rule violations for refs/heads/PINT-1842:
remote:
remote: - Changes must be made through a pull request.
remote:
To github.com:urbanairship/segment-docs.git
8a4e417..095c294 PINT-1842 -> PINT-1842
john @ cantankerous ~/projects/segment-docs (PINT-1842)
└─ $ ▶

Copy link
Contributor Author

@stayseesong stayseesong Jun 29, 2023

Choose a reason for hiding this comment

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

@jumpingGrendel that's strange, not sure why you're unable to edit the PR... Are the changes in here what you want to add to the docs? Just making sure, you don't want to include the Named User ID section?

Copy link
Contributor

Choose a reason for hiding this comment

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

no, sorry, I corrected it here.

I can't edit your new branch because it's in the segmentio repo. As I understand it, I develop on a remote fork and create a PR from there, that way I can continue to edit/update as needed until you (the repo admin) decide to merge it.

Copy link
Contributor

Choose a reason for hiding this comment

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

hi @jumpingGrendel. Apologies for the delays and disconnect here. We've run into similar issues before wherein our GitHub org does not work nicely with the GitHub org of our partners.

I will manually add your updates from the commit here

@netlify
Copy link

netlify bot commented Jun 29, 2023

Deploy Preview for segment-docs ready!

Name Link
🔨 Latest commit b53d3f9
🔍 Latest deploy log https://app.netlify.com/sites/segment-docs/deploys/649df8b44443810008f75d49
😎 Deploy Preview https://deploy-preview-4946--segment-docs.netlify.app/connections/destinations/catalog/actions-airship
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@markzegarelli
Copy link
Contributor

@jumpingGrendel I've updated the contents of the article, and incorporated available metadata (settings, actions, presets) from the API. Please have a look at the build preview here.

Please @ mention me directly if there is anything missing. If things look good, we'll deploy it.

@jumpingGrendel
Copy link
Contributor

Thanks @markzegarelli ! Only thing missing is my latest change to my remote fork: ad905be

@markzegarelli
Copy link
Contributor

Only thing missing is my latest change to my remote fork: ad905be

Hi @jumpingGrendel, I've included the content about Named Users here.

The content you for Additional Information is largely captured in the content for each action. For example, Custom Events. I will re-add your Additional Information section, but best practice would be to include the information here in the descriptions for the relevant fields in the code, for example here for the Custom Event Name field.

@jumpingGrendel
Copy link
Contributor

Thanks @markzegarelli it looks great, I am fine with it being merged if you are.

@stayseesong
Copy link
Contributor Author

Great, we'll get this merged in our deploy tmrw!

@stayseesong stayseesong merged commit d362402 into develop Jul 5, 2023
@stayseesong stayseesong deleted the PINT-1842 branch July 5, 2023 17:48
@github-actions
Copy link
Contributor

github-actions bot commented Jul 5, 2023

Thank you for your contribution! Your pull request is merged, but may take a day or two to appear on the site.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-integration Added a new source or destination
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants