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.

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

don't send emails or notifications for imports #2472

Closed
wants to merge 2 commits into from
Closed

don't send emails or notifications for imports #2472

wants to merge 2 commits into from

Conversation

BenLubar
Copy link
Contributor

@discoursebot
Copy link

You've signed the CLA, BenLubar. Thank you! This pull request is ready for review.

@BenLubar
Copy link
Contributor Author

@SamSaffron how's that?

@nlalonde
Copy link
Member

Github has deleted the conversation that I remember seeing here. The changes look sane to me, but Sam said this in the wtf discussion:

Add a site setting that disables external email.
or
Add a parameter to post creator to disable email creation. (used for import)

@SamSaffron
Copy link
Member

Yeah, no idea what happened to my comment :(

I did not want to invoke custom fields like this, it feels too magic, also you are disabling more than just emails there.

I think a better approach may be just to nip emails at the bud

Perhaps "disable email" site setting, that just disables all emails during import?

Regarding notifications it is more complex, I think they trigger user actions so you would have holes in your import, I think its better to let them through and then nuke the rows after import or something.

Backfilled import is so complicated.

@BenLubar
Copy link
Contributor Author

@SamSaffron @nlalonde I redid the patch because it was getting messy, and because Sam commented on one of the commits and not on the pull request itself, the comment was lost when I force-pushed the commits.

I can see a possible issue with blocking emails as a site setting, where emails could have been queued before the import and skipped incorrectly or queued near the end of the import and sent out after the setting was restored.

@riking
Copy link
Contributor

riking commented Jun 30, 2014

@BenLubar clearly the answer to that queuing problem is to prevent jobs from being queued during an import

(half-sarcasm)

@nlalonde
Copy link
Member

nlalonde commented Jul 2, 2014

I'm going to tackle the problem of not queuing emails from posts created during an import since I'm working on a couple of imports this week.

I would like to merge your first commit "set import_id on imported posts" because that's definitely useful. Do you want to make a separate PR for that? Or I can just do the changes myself and commit.

@nlalonde
Copy link
Member

nlalonde commented Jul 4, 2014

I added an option to PostCreator to prevent queuing of emails and some other jobs during import.

7d5d586

I'll close this.

@nlalonde nlalonde closed this Jul 4, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants