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

Add Team working now. AddPerson working. addPersonToTeam is still work in progress. #5

Merged
merged 2 commits into from
Dec 15, 2024

Conversation

DaleMcGrew
Copy link
Member

Add Team working now. AddPerson working. addPersonToTeam is still work in progress.

Copy link
Member

@SailingSteve SailingSteve Dec 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These environment names make my head spin, and they will make other developer's heads spin. That is most of why I made that WeVotePorts.md table.

WE_VOTE_HOSTNAME: is confusing. That is why I read the comments and changed it in the wrong direction.
We really have 4 servers in play...

The confusion's root is calling the "wevote react voter facing webapp" the "WebApp" and the "Python voter API server" the "WeVoteServer". (When even the webapp is a server that runs via nginix/express.)

Could we change the "weconnect" project name to "weconnect-client" or "weconnect-webapp"?

I know this is going to make for some long names, but let's standardize env variables (for remote servers) as:

  1. VOTER_FRONT_END_APP ("WebApp")
  2. STAFF_FRONT_END_APP (weconnect)
  3. VOTER_API_SERVER (WeVoteServer Python)
  4. STAFF_API_SERVER (weconnect-server)

Then in this file, for example
WE_VOTE_URL_PROTOCOL becomes VOTER_FRONT_END_APP_PROTOCOL
the others become...
VOTER_FRONT_END_APP_HOSTNAME
VOTER_FRONT_END_APP_PORT
etc

Then the env names
PROTOCOL:
HOSTNAME:
PORT:
are unambiguous since they always refer to "this" server, no matter what repository you are in.
The remote names with VOTER_FRONT_END_ or VOTER_API_ or STAFF_FRONT_END_ or STAFF_API_ are also unambiguous and and can be consistent in all 4 repositories.

Copy link
Member

@SailingSteve SailingSteve Dec 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API Calming is a horrible hack, that I put in because there were so many useless WebApp api calls firing off, that it was too overwhelming to track them down. Instead of API calming, lets carefully watch api calls with each new page, and even more closely watch cascading apis that come from stores.

Like on this file pair there has to be a way to avoid calling TeamActions.teamListRetrieve(); for every update of the store. Is there one bit of info we are looking for that we can poll for, before firing off the list retrieve?

If we just copy a bunch of webapp code, without carefully thinking through and improving each file, weconnect will start off inheriting the problems of WebApp. That is why I am always so negative about copying -- copying looks like fast progress, but usually brings technical debt into a new clean place. I would prefer to copy nothing until it can be actually used in the new codebase by running code.

Copy link
Member

@SailingSteve SailingSteve Dec 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be perfectly fine, but let's hold off big investments in this until I get user integrated into person with authentication going, and with api auth.

Copy link
Member

@SailingSteve SailingSteve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made some comments, if you are ok with the environment variable name changes I can make them on my next checkin.

@DaleMcGrew
Copy link
Member Author

Hi Steve, thank you for your review. I'm ok with your proposed environment variable names. I will change the repository "weconnect" to "weconnect-client" early tomorrow morning unless you have any objections.

AddPersonForm isn't meant to be a way to add all of a person's information -- just those three critical variables to set up the account. After we get the UX team to help with some designs, we will be adding ways for volunteers to update their own data, or the onboarding team to add data as needed.

I think apiCalming is an elegant way to request data needed by many components, where and when it is needed, without convoluted messaging to try to centralize requests for data. Do you have examples of what you would think of as an elegant way to request data needed by components that are spread around the interface?

@DaleMcGrew DaleMcGrew merged commit db41575 into wevote:develop Dec 15, 2024
0 of 2 checks passed
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