Canned Responses/Interactions questions disappear randomly from copied campaigns

[ErikKirkstein]

When copying a campaign in Spoke, sometimes the canned responses and/or interactions messages will be blank in the copy despite the original campaign having them filled, meaning they disappeared during the copying process. This appears to happen randomly, but it's possible there is a pattern (every 10th copy, for example). I just have not been able to adequately track the intervals.

This issue has been ongoing, but seems to have become worse in the past few days.

Link to copy that shows this issue: https://spoke.moveon.org/admin/3/campaigns/6672/edit

[joemcl]

hi @ErikKirkstein and @shakalee14 @schuyler1d - wondering if the above is fixed with #1107 , or if not if the cause is similar?

[joemcl]

hey @schuyler1d is this the same as #854 ?

[schuyler1d]

It certainly seems like it, right?

[schuyler1d]

This is likely an issue with this code:
https://github.com/MoveOnOrg/Spoke/blob/main/src/server/api/schema.js#L623-L636

What's happening is that some of the promises are not resolving before the request completes and then they are either cut off (due to VM policies) -- what we want to do is make sure that the whole set of actions are done synchronously.

While forEach-style code is more idiomatic it interacts badly with async. If someone works on this, likely the goal is to rewrite this in awaiting the cannedresponses query, and then iterating with a old-style for (....) { /*copy object*/ await ....save() }
While it is possible to use forEach with await, I'd suggest that we should keep it for..... for better readability and fewer footguns for future modifications.

[filafb]

I'll take a look at this issue!

[filafb]

Hi @schuyler1d! Sorry for taking long to work on this. Just opened a PR #1175 to this issue. I couldn't replicate the issue locally, but the tests I wrote were able to catch the race condition for copying canned responses and interactions. Lmk what you think.

[ibrand]

Hey @schuyler1d, do we think that this can be closed judging from @filafb's comment? ^

[schuyler1d]

I think we should wait until @filafb 's PR is merged into main, but agree on that being necessary and sufficient.

[shakalee14]

@ErikKirkstein - let me know if you still see this issue in the next few weeks? It's deployed in our instance.

[ErikKirkstein]

Yes, @shaka Lee <shaka.lee@moveon.org> , it did happen again this morning.

…

On Fri, Sep 13, 2019 at 10:03 AM Shaka Lee ***@***.***> wrote: @ErikKirkstein <https://github.com/ErikKirkstein> - let me know if you still see this issue in the next few weeks? It's deployed in our instance. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <https://github.com/MoveOnOrg/Spoke/issues/1141?email_source=notifications&email_token=AHVHK7K3WB7OL2Q3MIGCIXLQJOMULA5CNFSM4HXOVIF2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6VDVAI#issuecomment-531249793>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHVHK7NGMXH4KJWMWAO4OUDQJOMULANCNFSM4HXOVIFQ> .

-- Erik Kirkstein *he/him/his* Text Team Campaigns Director