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.

Sign up for GitHub

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

Jump to bottom

#556 support transactional sms #558

Merged
merged 29 commits into from
Nov 30, 2022
Merged

#556 support transactional sms #558

merged 29 commits into from
Nov 30, 2022

Conversation

JoernBerkefeld
Copy link
Contributor

@JoernBerkefeld JoernBerkefeld commented Nov 22, 2022 •
edited
Loading

important

todo:

  • during postRetrieve and preDeploy, check if dependent metadata is available
  • check for HTML in SMS body and warn user about it

@JoernBerkefeld JoernBerkefeld added the enhancement New feature or request; requires increasing the minor version of mcdev. Jira issue-type "Story" label Nov 22, 2022
@JoernBerkefeld JoernBerkefeld added this to the 4.2.0 milestone Nov 22, 2022
@JoernBerkefeld JoernBerkefeld self-assigned this Nov 22, 2022
@JoernBerkefeld JoernBerkefeld linked an issue Nov 22, 2022 that may be closed by this pull request
[FEATURE] support Transactional SMS #556
Closed
@JoernBerkefeld JoernBerkefeld mentioned this pull request Nov 22, 2022
Closed
1 task
@JoernBerkefeld JoernBerkefeld mentioned this pull request Nov 24, 2022
Merged
4 tasks
DougMidgley
DougMidgley requested changes Nov 24, 2022
View reviewed changes
Copy link
Contributor

@DougMidgley DougMidgley left a comment

Choose a reason for hiding this comment

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

No test classes :) esp given our test instance doesnt have sms this will be essential for regression testing

lib/metadataTypes/TransactionalMessage.js Outdated Show resolved Hide resolved
@JoernBerkefeld
#556: fix transactionalMessage UPDATE
b26fd74 
definitionKey must be in URL, not in body
@JoernBerkefeld
#556: added unit test: retrieve transactionalSMS
b1bbe49
@JoernBerkefeld
#556: added unit test: deploy (create + update) transactionalSMS
c230f48 
potentially fixed a long standing issue that should have prevented saving deploy results to retrieve folder
@JoernBerkefeld
#556: create-test files renamed
04b776b
@JoernBerkefeld
#556: confirm actual SQL looks correct after query deploy
ebf39ee
@JoernBerkefeld
#556: allow testing nested files as well
bfe4542 
e.g. sql, ssjs, ...
@JoernBerkefeld
#573:
c8bdf97
@JoernBerkefeld
#556: add chai-files to compare nested code; add transactionalSMS tem…
4323e82 
…plating test; enhance query templating test
@JoernBerkefeld
#556: fix how we test deploys
dbeb68d 
before, we would always hardlink the deploy folder which could have interfered with other tests
@JoernBerkefeld
#556: fix dataExtension tests for new markets
21b4f20
@JoernBerkefeld JoernBerkefeld linked an issue Nov 29, 2022 that may be closed by this pull request
[TASK] add mocha test for SQL file generated by r, bd, bt for queries #573
Closed
@JoernBerkefeld
Copy link
Contributor Author

should be ready for review now.

DougMidgley
DougMidgley requested changes Nov 30, 2022
View reviewed changes
lib/metadataTypes/MetadataType.js Outdated Show resolved Hide resolved
lib/metadataTypes/Query.js Outdated Show resolved Hide resolved
lib/metadataTypes/TransactionalMessage.js Outdated Show resolved Hide resolved
lib/metadataTypes/TransactionalSMS.js Outdated Show resolved Hide resolved
test/transactionalSMS.test.js Outdated Show resolved Hide resolved
test/utils.js Outdated Show resolved Hide resolved
test/utils.js Outdated Show resolved Hide resolved
JoernBerkefeld and others added 2 commits November 30, 2022 15:11
@JoernBerkefeld @DougMidgley
#556 Apply suggestions from code review
084c645 
Co-authored-by: Doug Midgley <douglasmidgley@gmail.com>
@JoernBerkefeld
#556: code review fixes
0291588
@JoernBerkefeld
Copy link
Contributor Author

all changes applied as requested. proceeding with merge as agreed

@JoernBerkefeld JoernBerkefeld merged commit 549b58d into develop Nov 30, 2022
@JoernBerkefeld JoernBerkefeld deleted the feature/556-support-transactional-sms branch November 30, 2022 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DougMidgley DougMidgley DougMidgley requested changes

Assignees

@JoernBerkefeld JoernBerkefeld

Labels
enhancement New feature or request; requires increasing the minor version of mcdev. Jira issue-type "Story"
Projects
mcdev
Status: Done
Milestone
4.2.0
Development

Successfully merging this pull request may close these issues.

[TASK] add mocha test for SQL file generated by r, bd, bt for queries [FEATURE] support Transactional SMS
2 participants
@JoernBerkefeld @DougMidgley