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

Candidate Invitations Details #164

Open
4 of 8 tasks
ddfridley opened this issue Jul 9, 2022 · 32 comments
Open
4 of 8 tasks

Candidate Invitations Details #164

ddfridley opened this issue Jul 9, 2022 · 32 comments
Labels

Comments

@ddfridley
Copy link
Contributor

ddfridley commented Jul 9, 2022

  • create a separate sendCandidateInvitations methods in app/components/get-election-info and emit the send-candudate-invitations api call
  • We need a positive feedback way to debounce the send button for invitations. After the user hits the button, it needs to be disabled until there is a positive response back from the server that the invitations have been sent (or not if there's an error).
  • extract reasonsNotReadyForCandidateRecorders out of app/socket-apis/create-candidate-recorders and make it available to the UI, probably through election status methods. In the UI, use this check to enable/disable the generate button, and display the error messages if there are any.
  • Add more details checks to reasonNotReady ... including:
    • Moderator submission available
    • All candidates have an office (and email and name)
  • Show the list of sent dates of invitations. [invitations might get sent more than once]. this is electionObj.moderator.invitations and the date is encoded in the _id use ObjectId(_id).getTimestamp() to get the date
  • when undebatesFromTemplateAndRows return in create-candidate-recorders, merge the new columns that show up inRowObjs into the candidate table. (this include the recorder link and the viewer link).
@ddfridley ddfridley added React V1 Req Required for Version 1 Release labels Jul 9, 2022
@nicholas-terry
Copy link
Collaborator

Break up into separate issues for multiple team members to work on?

NOTE

@ddfridley
Copy link
Contributor Author

@ice1080 I have the first one in a repo I will check in soon.

@ice1080 ice1080 self-assigned this Aug 18, 2022
@ice1080
Copy link
Collaborator

ice1080 commented Sep 7, 2022

@ddfridley for when you get back from vacation:
The third item of extracting reasonsNotReadyForCandidateRecorders into status methods says that it's for the Generate button. The only generate button in the code base is for the moderator recorder button and it already has a method for determining if it's ready for generation. Or perhaps I'm misunderstanding this and this is all for the Send Invites button?

@ddfridley
Copy link
Contributor Author

ddfridley commented Sep 7, 2022 via email

@ice1080
Copy link
Collaborator

ice1080 commented Sep 7, 2022

No problem. Is the other person just taking the third item of extracting? Or this whole issue?

@Shakadeliks
Copy link
Contributor

@ice1080 Just trying to find my way through the tasks step by step, so just undertaking the third item for now.

@ddfridley
Copy link
Contributor Author

@ice1080 Can you think of anything for this one:

Show the list of sent dates of invitations. [invitations might get sent more than once]. this is electionObj.moderator.invitations and the date is encoded in the _id use ObjectId(_id).getTimestamp() to get the date

And it says moderator.invitations but it should be about candidades.[candidateId].invitations

It's needs some UI design as well as a code - but minimal

@ice1080
Copy link
Collaborator

ice1080 commented Sep 7, 2022

Ya I can take that part @ddfridley

@ice1080
Copy link
Collaborator

ice1080 commented Sep 8, 2022

Hey @ddfridley, a couple questions on this one whenever you get a chance.

  1. Wanting to confirm that we are updating the candidate table for this list of invite sent dates? The current Invite Status column?
  2. You mentioned getting the date encoded in the _id. Is that preferred over using the invite object sentDate field?

@ddfridley
Copy link
Contributor Author

@ice1080
1-Not sure if your question is about the front end or the schema:
schema:
Invites are in candidades.[candidateId].invitations see https://github.com/EnCiv/undebate-ssp/wiki/electionObj-data-schema
invitations is auto generated by merging in those Iotas in getElectionDocs. It might be messy to try to render that in the candidate-table as is.
Front End:
Right now the submissions page looks like this. Showing for one candidate that invitation has been sent, but they haven't recorded. How do we show on this page that date/time(s) that the invitation was last sent. Maybe it's enough to show only the last, or maybe we can show a list. There may be multiple candidates in the Invite Send state, with different sent dates.

image

2-Good point. Use sentDate.

@ice1080
Copy link
Collaborator

ice1080 commented Sep 8, 2022

Ah got it. I was assuming you meant the Election Table page to do something like this:
image
I can also do both pages fairly easily by just making the code common

@ddfridley
Copy link
Contributor Author

Both pages sounds fine. Looks good. Its starting to look like the two pages should be combined - but later.

@ddfridley
Copy link
Contributor Author

ddfridley commented Sep 8, 2022

@ice1080 - an alternative idea, would it be hard to change the '<Envelope Icon>Sent' Submission status to '<Envelope Icon>Sent 210 days ago` and the not having a special column.

@ddfridley
Copy link
Contributor Author

@ice1080 Oh - I see the Send Reminders is only on the candidate-table page. Back to the first idea.

@ice1080
Copy link
Collaborator

ice1080 commented Sep 11, 2022

@ddfridley I believe it will be quite a bit of work and hacky code to get the Submissions page to have this list of invites without some pretty major refactoring. Since you talked about eventually combining them, would we be alright if this only goes to the Election Table page for now?

@ice1080
Copy link
Collaborator

ice1080 commented Sep 11, 2022

How does something like this look? You're seeing a tooltip that shows up on hover. I also renamed the column header.
image

@ddfridley
Copy link
Contributor Author

@ice1080 looks great. Good enough to have this for now, worry about submissions page in another iteration.

@ice1080
Copy link
Collaborator

ice1080 commented Sep 14, 2022

Hey @ddfridley I'm working on the create-candidate-recorders merging of undebatesFromTemplateAndRows result, but have an issue that is blocking me from being able to test it. This file assumes that the moderator is done recording, but I can't seem to finish recording for the moderator. When recording (in a different browser than the undebates creator), I am able to record all of the videos and sign up as the moderator, but when I try to Post, the upload percentage hangs, and I see this in the logs. Any idea what else I could be missing for successful uploading of recordings?

[1] [2022-09-13T22:26:26.262] [INFO] node - {
[1]   signUp: {
[1]     email: 'ice1080+moderator@gmail.com',
[1]     firstName: 'Mr',
[1]     lastName: 'Moderator'
[1]   }
[1] }
[1] [2022-09-13T22:26:26.328] [INFO] browser - 2022-09-14T04:26:26.326Z Undebate.onUserLogin {
[1]   socketId: 'UvhU82niZ3TJLMiRAAAJ',
[1]   userId: '632157f2816cc38ae029c170'
[1] }
[1] [2022-09-13T22:26:32.953] [INFO] browser - 2022-09-14T04:26:32.948Z Undebate.onUserUpload {
[1]   socketId: 'UvhU82niZ3TJLMiRAAAJ',
[1]   userId: '632157f2816cc38ae029c170'
[1] }
[1] [2022-09-13T22:26:32.959] [INFO] browser - 2022-09-14T04:26:32.952Z createParticipant.onUserUpload {
[1]   socketId: 'UvhU82niZ3TJLMiRAAAJ',
[1]   userId: '632157f2816cc38ae029c170'
[1] }
[1] [2022-09-13T22:26:32.962] [ERROR] node - caught error in streamUploadVideo, continuing

@ice1080
Copy link
Collaborator

ice1080 commented Sep 14, 2022

I was able to make a bit more progress with some heavy db hacking. Through this work, I noticed what I believe is a defect in create-moderator-recorder:

if (rowObjs[0].viewer_url) paths.push(rowObjs[0].viewer_url.replace(process.env.HOSTNAME + '/', ''))
if (rowObjs[0].recorder_url) paths.push(rowObjs[0].recorder_url.replace(process.env.HOSTNAME + '/', ''))

The viewer and recorder url start with http://localhost:3011/... in mine, and HOSTNAME is equal only to localhost:3011 so it can't find the iotas to merge. Within create-candidate-recorders.js I changed this to doing a replace like this instead: replace(process.env.HOST_NAME, ''). This is either a defect that needs to be fixed in create-moderator-recorder, or something that I got backwards with my local variables that we need to document.

Can you confirm the following for setting up a local env and I'll add it to the README.md? Should we have option 1:

export HOSTNAME=localhost:3011
export HOST_NAME="http://${HOSTNAME}"

or option 2:

export HOST_NAME=localhost:3011
export HOSTNAME="http://${HOSTNAME}"

@ddfridley if you can just run echo $HOSTNAME && echo $HOST_NAME on your local dev computer and paste the output here I believe that should clear this up.
Thanks!

@ddfridley
Copy link
Contributor Author

@ice1080 I applaud your heavy debugging skills! I see the defect you are talking about.
On my machine HOST_NAME is "localhost:3011/" but I see in the bashrcsetup file it puts the http:// in front.

We should deprecate the use of HOST_NAME in favor of HOSTNAME. In the undebate repo it is only used in tools and not in server code.

But for this issue, the real reason for that code is that we need to get to the path part. So I figured this out, which would be a more general solution:

if (rowObjs[0].viewer_url)  paths.push(new URL(rowObjs[0].viewer_url).pathname)
if (rowObjs[0].viewer_url)   paths.push(new URL(rowObjs[0].recorder_url).pathname)

Do you want to make this change in your branch? (I haven't tested it).

@ddfridley
Copy link
Contributor Author

If you do make the change, move the two lines under the try block below them. new URL can through an error if the url is not valid.

@ice1080
Copy link
Collaborator

ice1080 commented Sep 14, 2022

Ok we might need to look at the code because I've seen usages of both HOSTNAME and HOST_NAME which is part of why it's so confusing.
And ya I like your general solution, I'll push that change for both candidate and moderator recorders on my branch.

@ddfridley
Copy link
Contributor Author

ddfridley commented Sep 14, 2022

In app/lib/viewer-recorder-template.js I want to add:

const scheme = require('../lib/scheme')
const hostName = process.env.HOSTNAME ? scheme() + process.env.HOSTNAME : 'https://cc.enciv.org'

and delete the previous hostName assignment.

That's all the HOST_NAME in this repo.

@ddfridley
Copy link
Contributor Author

Here's a potential bug in my URL() idea. new URL('http://localhost:3011/hanna/banana').pathname is ''/hanna/banana'' asexpected, but new URL('localhost:3011/hanna/banana').pathname is '3011/hanna/banana' which seems like bug in URL() to me.

But as long as we are careful to put the schema in front of HOSTNAME than we are okay.

@ddfridley
Copy link
Contributor Author

const scheme = require('../lib/scheme') 

should be

import scheme from '../lib/scheme'

in above. I've made this change in my repo.

@ice1080
Copy link
Collaborator

ice1080 commented Sep 14, 2022

Just confirming, the viewer-recorder-template.js file above that you mentioned, that's where the email invites get created at?
That was one place I noticed both hostname and host_name getting used.

@ddfridley
Copy link
Contributor Author

viewer-recorder-template.js is the class for creating the template that is passed to undebatesFromTemplateAndRows to create the iotas that are the recorders for the candidates, and the viewer for the voter. Each iota gets a unique URL which is what you are getting back in rowObj.viewer_url and rowObj.recorder_url. The recorder URLs are sent in the email invites.

@ice1080
Copy link
Collaborator

ice1080 commented Sep 14, 2022

Ok I'll test with these changes then and make sure that it fixes the urls in the email. I would imagine it has to since there seem to be no other references to host_name

@ice1080
Copy link
Collaborator

ice1080 commented Sep 15, 2022

PR created.. As mentioned this only closes the last two tasks of this issue, not the whole issue. Let me know if you test this and it doesn't work, I wasn't able to locally get recording posts to work. Thanks!

@nicholas-terry
Copy link
Collaborator

@ddfridley Please break the remaining tasks into separate issues so we can close the user visible tasks.

@ddfridley
Copy link
Contributor Author

The remaining tasks here are not seen by the uses so I am removing the V1Req tab

@ddfridley ddfridley removed the V1 Req Required for Version 1 Release label Sep 27, 2022
@ddfridley
Copy link
Contributor Author

npm run test app/socket_apis/test/create-create-candidate-recorders.js

@ice1080 ice1080 removed their assignment Sep 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants