-
Notifications
You must be signed in to change notification settings - Fork 17
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
feat(exports)!: export multiple campaigns #1641
base: main
Are you sure you want to change the base?
Conversation
A possible follow-up issue: sending one email per bulk export operation. This was requested by @kohlee when speccing out the task originally, but (to the best of my knowledge) the way we currently run tasks via the I spent a half-day trying to make it work with the current configuration, but I think enabling this functionality might require a larger refactor to our job-handling architecture. Happy to spin out an issue and do some more speccing there, if folks think it's worth it - @bchrobot @hiemanshu thoughts? |
One other note - initially @kohlee requested being able to export multiple van campaigns, but @hiemanshu thought it might be too much work. I think we could handle multiple van exports using a similar approach as we use here for spoke exports, unless I'm missing something. @hiemanshu I feel like it might be another one or two hours work, unless I'm missing something? |
A follow up issue sounds right to me so as to unblock this. But I defer to @kohlee on how much of a usability issue multiple emails is. My thoughts on implementation are not that different from what you're currently doing with the new Re: VAN campaigns, maybe punt that to a follow up issue (again just to unblock this being merged and released) with some detail about what minimal change set is needed to also support VAN exports? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 larger question about job tables at the end, and an assortment of smaller things, but nicee. Excited for the new layout 🥳
interface CampaignDetailsProps { | ||
id: string; | ||
description: string; | ||
creatorName: string | null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The field is nullable. So we should work with that assumption. Maybe inserting campaigns through API or something?
@henryk1229 Overall looks good! A few nit picks / suggestions. I would create a new PR on top of this, to unblock this from being merged, to send all exports in 1 email. |
@kennyycheng @ajohn25 this requires a one-line change to |
this sounds good to me just in case yeah 👍🏾 |
Description
This PR makes it possible for admins to export multiple campaigns at once. It also makes it possible for admins to filter campaigns by title, and introduces some design-related changes to the
CampaignList
view, to make it easier to incorporate the new functionality into theAdminCampaignList
component without overwhelming users.Motivation and Context
It closes #915
How Has This Been Tested?
Behaviorally
Screenshots (if appropriate):
Screen.Recording.2023-08-04.at.2.54.09.PM.mov
Untitled.mov
Documentation Changes
Documentation change TK
Checklist: