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

undebate-ssp send password#154 #33

Merged
merged 15 commits into from
Oct 24, 2022

Conversation

ice1080
Copy link
Contributor

@ice1080 ice1080 commented Oct 20, 2022

No description provided.

@ice1080 ice1080 mentioned this pull request Oct 20, 2022
3 tasks
@ddfridley
Copy link
Contributor

After a quick look:

There are some new files in app/tools that I think should be in app/lib. app/tools if for things that you would run from a command line. There is no app/lib in civil-server, yet, but there is in other repos.

Also, since send-in-blue-transactional has been moved into civil-server, can it be added to app/index.js so that we can (eventually) import it on the undebate-ssp side. Is there anything else that should go there?

I'll figure out how to load and test it tomorrow.

@ice1080
Copy link
Contributor Author

ice1080 commented Oct 20, 2022

@ddfridley Do you care if we are exporting the individual functions or would you prefer sib to be a class with all the functions on it?

@ice1080 ice1080 mentioned this pull request Oct 21, 2022
4 tasks
@ice1080
Copy link
Contributor Author

ice1080 commented Oct 21, 2022

I tested out the export of the sib stuff into undebate-ssp, but the main problem seems to be that SibGetTemplateId assumes that the email templates are local, which means that templates like moderator-invitation.html and candidate-invitation.html aren't being found. This could probably be refactored at some point, but I would say this is becoming a bit of scope creep at this point. For now the functions will continue to be exports in index.js, but it will require some refactoring on both sides to be able to use those. Finishing up one more thing on civil-server now.

@ice1080
Copy link
Contributor Author

ice1080 commented Oct 21, 2022

Alright that's pushed. At this point all three PR's should again be ready for test and/or merge. Let me know if you find anything else you'd like for these!

@ddfridley
Copy link
Contributor

ddfridley commented Oct 21, 2022 via email

@ddfridley
Copy link
Contributor

I'm checking this in without being able to run the cypress tests. On PC, this problem prevents cy.exec() from working: cypress-io/cypress#789 and on my MacBook Air, when I run cypress a blank electron window pops up, and nothing happens or can be done. I've tried upgrading to cypress 10 but still had that problem. Will battle more with cypress another time.

@ddfridley ddfridley merged commit 298079e into EnCiv:main Oct 24, 2022
@ice1080
Copy link
Contributor Author

ice1080 commented Oct 24, 2022

In case it makes you feel any better, I just switched to main, pulled the latest, and saw all the cypress tests passing.

@ice1080 ice1080 deleted the undebate-ssp-send-password#154 branch October 24, 2022 21:41
@ddfridley
Copy link
Contributor

ddfridley commented Oct 24, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants