From 5e2850a44beb3ce320366251bbe3c8c8a18b9953 Mon Sep 17 00:00:00 2001 From: Kristijan Mitrovic Date: Fri, 18 Sep 2020 12:06:35 +0000 Subject: [PATCH] Fix account deletion (#55) Merge branch 'master' into kris/delete_account Create AccountMessage in DB only if Account is still there Use exact versions of all submodules when building Set confirmation.welcome.account message to be quiet Move welcome account template to requests Merge branch 'master' into kris/delete_account # Conflicts: # bubble-web Merge branch 'master' into kris/delete_account # Conflicts: # bubble-web Merge branch 'master' into kris/delete_account # Conflicts: # bubble-server/src/main/resources/messages # bubble-web Fix account's refs deletion on account delete Try to fix timing of delete confirmation notice sent out Update web and messages Co-authored-by: jonathan Co-authored-by: Kristijan Mitrovic Reviewed-on: https://git.bubblev.org/bubblev/bubble/pulls/55 --- bin/first_time_setup.sh | 6 +--- bin/git_update_bubble.sh | 11 +++---- .../java/bubble/dao/account/AccountDAO.java | 5 ++++ .../dao/account/AccountInitializer.java | 2 +- .../account/message/AccountMessageDAO.java | 12 -------- .../bubble/model/account/AccountContact.java | 2 +- .../model/account/message/AccountMessage.java | 4 +-- .../StandardAccountMessageService.java | 30 +++++++++++++++++-- bubble-server/src/main/resources/messages | 2 +- .../resources/models/include/new_account.json | 2 +- .../models/include/referral_signup.json | 4 +-- .../models/tests/payment/pay_credit.json | 4 +-- .../pay_credit_refund_and_restart.json | 4 +-- .../tests/payment/recurring_billing.json | 4 +-- .../models/tests/promo/account_credit.json | 4 +-- .../models/tests/promo/first_month_free.json | 4 +-- .../models/tests/promo/multi_promo.json | 4 +-- .../tests/promo/referral_month_free.json | 8 ++--- 18 files changed, 62 insertions(+), 50 deletions(-) diff --git a/bin/first_time_setup.sh b/bin/first_time_setup.sh index d751664e..fb4db0d3 100755 --- a/bin/first_time_setup.sh +++ b/bin/first_time_setup.sh @@ -29,8 +29,7 @@ function die { BASE=$(cd $(dirname $0)/.. && pwd) cd ${BASE} -git submodule init || die "Error in git submodule init" -git submodule update || die "Error in git submodule update" +git submodule update --init --recursive || die "Error in git submodule update" pushd utils/cobbzilla-parent mvn install || die "Error installing cobbzilla-parent" @@ -50,9 +49,6 @@ for repo in ${UTIL_REPOS} ; do done popd -MESSAGES_REPO=bubble-server/src/main/resources/messages -pushd ${MESSAGES_REPO} && git checkout master && popd || die "Error installing ${MESSAGES_REPO}" - if [[ -z "${BUBBLE_SETUP_MODE}" || "${BUBBLE_SETUP_MODE}" == "web" ]] ; then INSTALL_WEB=web mvn -DskipTests=true -Dcheckstyle.skip=true clean package || die "Error building bubble jar" diff --git a/bin/git_update_bubble.sh b/bin/git_update_bubble.sh index 215e129e..b0f7e5c6 100755 --- a/bin/git_update_bubble.sh +++ b/bin/git_update_bubble.sh @@ -32,10 +32,10 @@ cd ${BASE} git fetch || die "Error calling git fetch" git pull origin master || die "Error calling git pull origin master" -git submodule update || die "Error in git submodule update" +git submodule update --init --recursive || die "Error in git submodule update" pushd utils/cobbzilla-parent -git fetch && git checkout master && git pull origin master && mvn install || die "Error updating cobbzilla-parent" +mvn install || die "Error installing cobbzilla-parent" popd UTIL_REPOS=" @@ -49,16 +49,13 @@ abp-parser pushd utils for repo in ${UTIL_REPOS} ; do if [[ ${FAST} -eq 1 ]] ; then - pushd ${repo} && git fetch && git checkout master && git pull origin master && mvn -DskipTests=true -Dcheckstyle.skip=true install && popd || die "Error updating ${repo}" + pushd ${repo} && mvn -DskipTests=true -Dcheckstyle.skip=true install && popd || die "Error installing ${repo}" else - pushd ${repo} && git fetch && git checkout master && git pull origin master && mvn -DskipTests=true -Dcheckstyle.skip=true clean install && popd || die "Error updating ${repo}" + pushd ${repo} && mvn -DskipTests=true -Dcheckstyle.skip=true clean install && popd || die "Error installing ${repo}" fi done popd -MESSAGES_REPO=bubble-server/src/main/resources/messages -pushd ${MESSAGES_REPO} && git fetch && git checkout master && git pull origin master && popd || die "Error updating ${MESSAGES_REPO}" - if [[ ${FAST} -eq 1 ]] ; then mvn -DskipTests=true -Dcheckstyle.skip=true clean package || die "Error building bubble jar" else diff --git a/bubble-server/src/main/java/bubble/dao/account/AccountDAO.java b/bubble-server/src/main/java/bubble/dao/account/AccountDAO.java index a0a714b3..8774ed9c 100644 --- a/bubble-server/src/main/java/bubble/dao/account/AccountDAO.java +++ b/bubble-server/src/main/java/bubble/dao/account/AccountDAO.java @@ -13,6 +13,7 @@ import bubble.dao.bill.AccountPaymentArchivedDAO; import bubble.dao.bill.BillDAO; import bubble.dao.cloud.BubbleDomainDAO; import bubble.dao.cloud.BubbleFootprintDAO; +import bubble.dao.cloud.BubbleNetworkDAO; import bubble.dao.cloud.CloudServiceDAO; import bubble.dao.device.DeviceDAO; import bubble.model.account.*; @@ -410,6 +411,10 @@ public class AccountDAO extends AbstractCRUDDAO implements SqlViewSearc configuration.getBean(AccountPaymentArchivedDAO.class).createForAccount(account); log.info("delete: starting to delete account-dependent objects - " + currentThread().getName()); + final BubbleNetworkDAO networkDAO = configuration.getBean(BubbleNetworkDAO.class); + final List networks = networkDAO.findByAccount(account.getUuid()); + networks.forEach(n -> networkDAO.delete(n.getUuid())); // deleting all networks dependency objects + configuration.deleteDependencies(account); log.info("delete: finished deleting account-dependent objects - " + currentThread().getName()); diff --git a/bubble-server/src/main/java/bubble/dao/account/AccountInitializer.java b/bubble-server/src/main/java/bubble/dao/account/AccountInitializer.java index 1c14e988..a9d7ad99 100644 --- a/bubble-server/src/main/java/bubble/dao/account/AccountInitializer.java +++ b/bubble-server/src/main/java/bubble/dao/account/AccountInitializer.java @@ -99,7 +99,7 @@ public class AccountInitializer implements Runnable { .setAccount(accountUuid) .setName(accountUuid) .setNetwork(thisNetwork.getUuid()) - .setMessageType(AccountMessageType.notice) + .setMessageType(AccountMessageType.request) .setAction(AccountAction.welcome) .setTarget(ActionTarget.account) .setContact(contact)); diff --git a/bubble-server/src/main/java/bubble/dao/account/message/AccountMessageDAO.java b/bubble-server/src/main/java/bubble/dao/account/message/AccountMessageDAO.java index c5442061..76a66cc7 100644 --- a/bubble-server/src/main/java/bubble/dao/account/message/AccountMessageDAO.java +++ b/bubble-server/src/main/java/bubble/dao/account/message/AccountMessageDAO.java @@ -166,9 +166,6 @@ public class AccountMessageDAO extends AccountOwnedEntityDAO { } public AccountMessage findOperationRequest(AccountMessage basis) { - if (basis.getAction() == AccountAction.welcome && basis.getTarget() == ActionTarget.account) { - return findWelcomeNotice(basis); - } return findByUniqueFields("account", basis.getAccount(), "name", basis.getName(), "requestId", basis.getRequestId(), @@ -177,15 +174,6 @@ public class AccountMessageDAO extends AccountOwnedEntityDAO { "target", basis.getTarget()); } - public AccountMessage findWelcomeNotice(AccountMessage basis) { - return findByUniqueFields("account", basis.getAccount(), - "name", basis.getName(), - "requestId", basis.getRequestId(), - "messageType", AccountMessageType.notice, - "action", AccountAction.welcome, - "target", ActionTarget.account); - } - public List findOperationDenials(AccountMessage basis) { if (basis == null) { return Collections.emptyList(); diff --git a/bubble-server/src/main/java/bubble/model/account/AccountContact.java b/bubble-server/src/main/java/bubble/model/account/AccountContact.java index df97cd13..5cc2e0af 100644 --- a/bubble-server/src/main/java/bubble/model/account/AccountContact.java +++ b/bubble-server/src/main/java/bubble/model/account/AccountContact.java @@ -208,7 +208,7 @@ public class AccountContact implements Serializable { if (!verified()) { if ( target == ActionTarget.account && ( (action == AccountAction.verify) // all verification-related messages are allowed to unverified - || (type == AccountMessageType.notice && action == AccountAction.welcome) // welcome is allowed to unverified + || (type == AccountMessageType.request && action == AccountAction.welcome) // welcome is allowed to unverified ) ) { log.info("isAllowed(" + message.getAction() + "): allowing "+type+" message to unverified contact: "+action); } else { diff --git a/bubble-server/src/main/java/bubble/model/account/message/AccountMessage.java b/bubble-server/src/main/java/bubble/model/account/message/AccountMessage.java index 60deb681..2bb184c5 100644 --- a/bubble-server/src/main/java/bubble/model/account/message/AccountMessage.java +++ b/bubble-server/src/main/java/bubble/model/account/message/AccountMessage.java @@ -97,8 +97,8 @@ public class AccountMessage extends IdentifiableBase implements HasAccount { public String templateName(String basename) { return getMessageType()+"/"+ getAction()+"/"+getTarget()+"/"+basename+".hbs"; } public long tokenTimeoutSeconds(AccountPolicy policy) { - // only requests and welcome message get tokens (welcome messages also verify the initial email address) - if (getMessageType() == AccountMessageType.request || (getMessageType() == AccountMessageType.notice && getAction() == AccountAction.welcome)) { + if (getMessageType() == AccountMessageType.request) { + // only requests get tokens switch (getTarget()) { case account: return policy.getAccountOperationTimeout()/1000; case network: return policy.getNodeOperationTimeout()/1000; diff --git a/bubble-server/src/main/java/bubble/service/account/StandardAccountMessageService.java b/bubble-server/src/main/java/bubble/service/account/StandardAccountMessageService.java index c6564958..c509d224 100644 --- a/bubble-server/src/main/java/bubble/service/account/StandardAccountMessageService.java +++ b/bubble-server/src/main/java/bubble/service/account/StandardAccountMessageService.java @@ -18,6 +18,7 @@ import bubble.model.account.message.handlers.*; import bubble.model.cloud.CloudService; import bubble.server.BubbleConfiguration; import lombok.Getter; +import lombok.NonNull; import lombok.extern.slf4j.Slf4j; import org.cobbzilla.util.collection.NameAndValue; import org.cobbzilla.util.string.StringUtil; @@ -47,7 +48,29 @@ public class StandardAccountMessageService implements AccountMessageService { @Autowired private CloudServiceDAO cloudDAO; @Autowired private BubbleConfiguration configuration; + /** + * If returns true, then email/sms will NOT be sent out for this AccountMessage object. + */ + private boolean isQuietMessage(@NonNull final AccountMessage message) { + if (message.getMessageType() == AccountMessageType.confirmation + && message.getAction() == AccountAction.welcome + && message.getTarget() == ActionTarget.account) { + // No need for confirmation message here. The end user received `request.welcome.account` message with email + // address verification link. When he clicked the link, then the request was approved. That was the only + // required approval, and so it was also confirmed at that moment. Hence, AccountMessage + // `confirmation.welcome.account` is created. But there no real need to send out email at this point to the + // same user. And so this other confirmation message is `quiet`. + return true; + } + return false; + } + @Override public boolean send(AccountMessage message) { + if (isQuietMessage(message)) { + log.info("send(" + message + "): message marked as quiet"); + return false; + } + final String accountUuid = message.getAccount(); final Account account = accountDAO.findByUuid(accountUuid); AccountPolicy policy = policyDAO.findSingleByAccount(accountUuid); @@ -140,16 +163,19 @@ public class StandardAccountMessageService implements AccountMessageService { final AccountMessage request = messageDAO.findOperationRequest(approval); if (request == null) throw invalidEx("err.approvalToken.invalid", "Request could not be found for approval: "+approval); final AccountPolicy policy = policyDAO.findSingleByAccount(account.getUuid()); - final AccountMessage confirm = messageDAO.create(new AccountMessage(approval).setMessageType(AccountMessageType.confirmation)); approval.setRequest(request); approval.setRequestContact(policy.findContactByUuid(approval.getRequest().getContact())); getCompletionHandler(approval).confirm(approval, data); + final AccountMessage confirm = new AccountMessage(approval).setMessageType(AccountMessageType.confirmation); if (approval.hasConfirmationTokensToRemove()) { final RedisService tokens = getConfirmationTokens(); for (String toRemove : approval.getConfirmationTokensToRemove()) tokens.del(toRemove); } - return confirm; + + // Write AccountMessage entity into database only if Account is still available in there + // (i.e. not removed with block_delete deletion policy) + return accountDAO.exists(confirm.getAccount()) ? messageDAO.create(confirm) : confirm; } else if (approvalStatus.ok()) { if (approval.hasConfirmationTokensToRemove()) { diff --git a/bubble-server/src/main/resources/messages b/bubble-server/src/main/resources/messages index 04db2238..6767812d 160000 --- a/bubble-server/src/main/resources/messages +++ b/bubble-server/src/main/resources/messages @@ -1 +1 @@ -Subproject commit 04db22382ddffa08b3f05c024603da75cdfb8b55 +Subproject commit 6767812db299ffb810c8d88586a6462c670bfda7 diff --git a/bubble-server/src/test/resources/models/include/new_account.json b/bubble-server/src/test/resources/models/include/new_account.json index 7d34c4b4..f04c4fd9 100644 --- a/bubble-server/src/test/resources/models/include/new_account.json +++ b/bubble-server/src/test/resources/models/include/new_account.json @@ -41,7 +41,7 @@ "store": "userInbox", "check": [ {"condition": "json.length == 1"}, - {"condition": "'{{json.[0].ctx.message.messageType}}' == 'notice'"}, + {"condition": "'{{json.[0].ctx.message.messageType}}' == 'request'"}, {"condition": "'{{json.[0].ctx.message.action}}' == 'welcome'"}, {"condition": "'{{json.[0].ctx.message.target}}' == 'account'"} ] diff --git a/bubble-server/src/test/resources/models/include/referral_signup.json b/bubble-server/src/test/resources/models/include/referral_signup.json index 82804b45..beebc188 100644 --- a/bubble-server/src/test/resources/models/include/referral_signup.json +++ b/bubble-server/src/test/resources/models/include/referral_signup.json @@ -37,12 +37,12 @@ "comment": "as root, check email inbox for welcome message", "request": { "session": "<>", - "uri": "debug/inbox/email/<>@example.com?type=notice&action=welcome&target=account" + "uri": "debug/inbox/email/<>@example.com?type=request&action=welcome&target=account" }, "response": { "store": "emailInbox", "check": [ - {"condition": "'{{json.[0].ctx.message.messageType}}' == 'notice'"}, + {"condition": "'{{json.[0].ctx.message.messageType}}' == 'request'"}, {"condition": "'{{json.[0].ctx.message.action}}' == 'welcome'"}, {"condition": "'{{json.[0].ctx.message.target}}' == 'account'"} ] diff --git a/bubble-server/src/test/resources/models/tests/payment/pay_credit.json b/bubble-server/src/test/resources/models/tests/payment/pay_credit.json index 87a5342d..53b8dd46 100644 --- a/bubble-server/src/test/resources/models/tests/payment/pay_credit.json +++ b/bubble-server/src/test/resources/models/tests/payment/pay_credit.json @@ -105,12 +105,12 @@ "comment": "as root, check email inbox for welcome message", "request": { "session": "rootSession", - "uri": "debug/inbox/email/test_user_credit@example.com?type=notice&action=welcome&target=account" + "uri": "debug/inbox/email/test_user_credit@example.com?type=request&action=welcome&target=account" }, "response": { "store": "emailInbox", "check": [ - {"condition": "'{{json.[0].ctx.message.messageType}}' == 'notice'"}, + {"condition": "'{{json.[0].ctx.message.messageType}}' == 'request'"}, {"condition": "'{{json.[0].ctx.message.action}}' == 'welcome'"}, {"condition": "'{{json.[0].ctx.message.target}}' == 'account'"} ] diff --git a/bubble-server/src/test/resources/models/tests/payment/pay_credit_refund_and_restart.json b/bubble-server/src/test/resources/models/tests/payment/pay_credit_refund_and_restart.json index 9b555d9b..1ccd55d6 100644 --- a/bubble-server/src/test/resources/models/tests/payment/pay_credit_refund_and_restart.json +++ b/bubble-server/src/test/resources/models/tests/payment/pay_credit_refund_and_restart.json @@ -44,12 +44,12 @@ "comment": "as root, check email inbox for welcome message", "request": { "session": "rootSession", - "uri": "debug/inbox/email/test_user_credit_refund_restart@example.com?type=notice&action=welcome&target=account" + "uri": "debug/inbox/email/test_user_credit_refund_restart@example.com?type=request&action=welcome&target=account" }, "response": { "store": "emailInbox", "check": [ - {"condition": "'{{json.[0].ctx.message.messageType}}' == 'notice'"}, + {"condition": "'{{json.[0].ctx.message.messageType}}' == 'request'"}, {"condition": "'{{json.[0].ctx.message.action}}' == 'welcome'"}, {"condition": "'{{json.[0].ctx.message.target}}' == 'account'"} ] diff --git a/bubble-server/src/test/resources/models/tests/payment/recurring_billing.json b/bubble-server/src/test/resources/models/tests/payment/recurring_billing.json index c02106a6..9285c2e6 100644 --- a/bubble-server/src/test/resources/models/tests/payment/recurring_billing.json +++ b/bubble-server/src/test/resources/models/tests/payment/recurring_billing.json @@ -44,12 +44,12 @@ "comment": "as root, check email inbox for welcome message", "request": { "session": "rootSession", - "uri": "debug/inbox/email/test_user_recurring@example.com?type=notice&action=welcome&target=account" + "uri": "debug/inbox/email/test_user_recurring@example.com?type=request&action=welcome&target=account" }, "response": { "store": "emailInbox", "check": [ - {"condition": "'{{json.[0].ctx.message.messageType}}' == 'notice'"}, + {"condition": "'{{json.[0].ctx.message.messageType}}' == 'request'"}, {"condition": "'{{json.[0].ctx.message.action}}' == 'welcome'"}, {"condition": "'{{json.[0].ctx.message.target}}' == 'account'"} ] diff --git a/bubble-server/src/test/resources/models/tests/promo/account_credit.json b/bubble-server/src/test/resources/models/tests/promo/account_credit.json index f6bb8f80..569881f7 100644 --- a/bubble-server/src/test/resources/models/tests/promo/account_credit.json +++ b/bubble-server/src/test/resources/models/tests/promo/account_credit.json @@ -22,12 +22,12 @@ "comment": "root: check email inbox for welcome message", "request": { "session": "rootSession", - "uri": "debug/inbox/email/account_credit_user@example.com?type=notice&action=welcome&target=account" + "uri": "debug/inbox/email/account_credit_user@example.com?type=request&action=welcome&target=account" }, "response": { "store": "emailInbox", "check": [ - {"condition": "'{{json.[0].ctx.message.messageType}}' == 'notice'"}, + {"condition": "'{{json.[0].ctx.message.messageType}}' == 'request'"}, {"condition": "'{{json.[0].ctx.message.action}}' == 'welcome'"}, {"condition": "'{{json.[0].ctx.message.target}}' == 'account'"} ] diff --git a/bubble-server/src/test/resources/models/tests/promo/first_month_free.json b/bubble-server/src/test/resources/models/tests/promo/first_month_free.json index 45605c5f..d76224d9 100644 --- a/bubble-server/src/test/resources/models/tests/promo/first_month_free.json +++ b/bubble-server/src/test/resources/models/tests/promo/first_month_free.json @@ -22,12 +22,12 @@ "comment": "as root, check email inbox for welcome message", "request": { "session": "rootSession", - "uri": "debug/inbox/email/test_user_1mo_free@example.com?type=notice&action=welcome&target=account" + "uri": "debug/inbox/email/test_user_1mo_free@example.com?type=request&action=welcome&target=account" }, "response": { "store": "emailInbox", "check": [ - {"condition": "'{{json.[0].ctx.message.messageType}}' == 'notice'"}, + {"condition": "'{{json.[0].ctx.message.messageType}}' == 'request'"}, {"condition": "'{{json.[0].ctx.message.action}}' == 'welcome'"}, {"condition": "'{{json.[0].ctx.message.target}}' == 'account'"} ] diff --git a/bubble-server/src/test/resources/models/tests/promo/multi_promo.json b/bubble-server/src/test/resources/models/tests/promo/multi_promo.json index da3456b9..b3ca71b8 100644 --- a/bubble-server/src/test/resources/models/tests/promo/multi_promo.json +++ b/bubble-server/src/test/resources/models/tests/promo/multi_promo.json @@ -50,12 +50,12 @@ "comment": "root: check email inbox for welcome message for referring user", "request": { "session": "rootSession", - "uri": "debug/inbox/email/test_user_referring_multi@example.com?type=notice&action=welcome&target=account" + "uri": "debug/inbox/email/test_user_referring_multi@example.com?type=request&action=welcome&target=account" }, "response": { "store": "emailInbox", "check": [ - {"condition": "'{{json.[0].ctx.message.messageType}}' === 'notice'"}, + {"condition": "'{{json.[0].ctx.message.messageType}}' === 'request'"}, {"condition": "'{{json.[0].ctx.message.action}}' === 'welcome'"}, {"condition": "'{{json.[0].ctx.message.target}}' === 'account'"} ] diff --git a/bubble-server/src/test/resources/models/tests/promo/referral_month_free.json b/bubble-server/src/test/resources/models/tests/promo/referral_month_free.json index 4bc94f04..fd418019 100644 --- a/bubble-server/src/test/resources/models/tests/promo/referral_month_free.json +++ b/bubble-server/src/test/resources/models/tests/promo/referral_month_free.json @@ -45,12 +45,12 @@ "comment": "root: check email inbox for welcome message for referring user", "request": { "session": "rootSession", - "uri": "debug/inbox/email/test_user_referring_free@example.com?type=notice&action=welcome&target=account" + "uri": "debug/inbox/email/test_user_referring_free@example.com?type=request&action=welcome&target=account" }, "response": { "store": "emailInbox", "check": [ - {"condition": "'{{json.[0].ctx.message.messageType}}' == 'notice'"}, + {"condition": "'{{json.[0].ctx.message.messageType}}' == 'request'"}, {"condition": "'{{json.[0].ctx.message.action}}' == 'welcome'"}, {"condition": "'{{json.[0].ctx.message.target}}' == 'account'"} ] @@ -119,12 +119,12 @@ "comment": "as root, check email inbox for welcome message", "request": { "session": "rootSession", - "uri": "debug/inbox/email/test_user_referred_free@example.com?type=notice&action=welcome&target=account" + "uri": "debug/inbox/email/test_user_referred_free@example.com?type=request&action=welcome&target=account" }, "response": { "store": "emailInbox", "check": [ - {"condition": "'{{json.[0].ctx.message.messageType}}' == 'notice'"}, + {"condition": "'{{json.[0].ctx.message.messageType}}' == 'request'"}, {"condition": "'{{json.[0].ctx.message.action}}' == 'welcome'"}, {"condition": "'{{json.[0].ctx.message.target}}' == 'account'"} ]