Skip to content
This repository has been archived by the owner on Jul 17, 2020. It is now read-only.

Adding a new player to a league does not update the player list #80

Open
makkoli opened this issue Mar 7, 2018 · 19 comments
Open

Adding a new player to a league does not update the player list #80

makkoli opened this issue Mar 7, 2018 · 19 comments

Comments

@makkoli
Copy link
Contributor

makkoli commented Mar 7, 2018

Players tab > Manually adding a player to league

After successful insertion, the roster does not update with the new player. It is being added to the list of unapproved player registrations. If the admin manually adds a player it should be immediately added to the list of all players.

@afranco07
Copy link
Contributor

I want to try and work on this

@afranco07
Copy link
Contributor

Hey I have this pretty much done, however I can't get the state to update the team roster without refreshing the page. Any help on which reducer to use for getting the latest team/league info?

@IsaacRaymond
Copy link
Contributor

IsaacRaymond commented Apr 14, 2018

@afranco07 can you show what you have for this fix? I'm not an expert, but from my experience with react, proper changes to the state should automatically call the component to re-render

edit: Sorry, re-read your post. You're looking to change the state, not to get the component to re-render. I'll look and see if I can find which reducer is needed!

@afranco07
Copy link
Contributor

afranco07 commented Apr 17, 2018

@IRGames yea I'm having trouble updating the state of the roster. There is a reducer UPDATE_TEAM under teams.js in the reducers folder, but I'm not sure if it is working correctly. And like I said, refreshing the page updates the state correctly

Here is some code I wrote attempting to update state. This is in the file createPlayer.js in this location client/src/actions/players/ in the creatPlayer(form, dispatch, props) function, starts on line 31. I may be missing something:

if ( team && team.teamId) {
    const leagueID = data.leagueId;
    axios.get(`${ROOT_URL}/league/fetch/${leagueID}`)
        .then(leagueData => {
            dispatch({
                type: EDIT_LEAGUE,
                payload: {
                    [leagueID]: { ...leagueData.data }
                }
            });
        });
}

@IsaacRaymond
Copy link
Contributor

IsaacRaymond commented Apr 18, 2018

When you try adding a player, does it also "break" the app, like in these pictures?
image
After clicking "Add Player"
image

@IsaacRaymond
Copy link
Contributor

IsaacRaymond commented Apr 19, 2018

I'm really new to this app and fairly new to React, in general, but here's what I've got after studying this:

The type of state change that you're dispatching changes the entire league. I've tried to figure out why it causes the above glitch, but the reason hasn't come to me yet. I'm sure it has something to do with the fact that we're changing the entire state of the league when we're accessing a child of the league.

The variable we need to change in the state is "data.pending." Since we're the admin, this person isn't pending admin approval. It should be set to "true" instead of the default value of "false".

I've added one new line of code beneath line 20 of createPlayer.js:
data.pending = false;
I know a reducer is only supposed to change the state, nothing extra. Is there a special convention about actions? Am I doing something wrong by adding this line of code?

If you add that one line, the problem is fixed and the player is automatically approved.

@afranco07
Copy link
Contributor

@IRGames yes you are correct about the data.pending = false; and I already have some logic that checks if you are the admin for setting that. The problem is I need to somehow update the roster to include the new player you just created (without refreshing the page, just the state). If I go to the team roster, the player is not shown. However if I refresh the page then the player does show up in the roster. So there must be a way to get the state to update the roster, I just haven't found/figured it out yet.

Also it doesn't break like in the pictures you have above. There is an error that gets printed to the console but I don't exactly remember what it said. I'll be able to get that error for you tomorrow if you want.

I did get that error in the pictures before while working on something else. It happened when I was sending the wrong payload into the Reducer. Double check the data you're sending in

@IsaacRaymond
Copy link
Contributor

Ok! If you could, I'd like to see the error. When my app crashed on my end, no error appeared.

I noticed one of the "To Dos" for the app was to "Finish writing out roles" (admin, coach, etc). When you get a chance, could you show me where you added the logic to check for admin? I was wondering if/how the app is currently set up to check a user's role. Thanks for talking things out with me here!

@afranco07
Copy link
Contributor

@IRGames I new to react and redux as well so this might not be the best way to do it but, here's a little snippet on how I am getting the users role. I get the current users email from redux state and find them under the staff array in the redux state (I save it into user variable). Then I check if the users role is Admin using the ternary if statement.

function mapStateToProps(state, ownProps) {
	// Getting the users role
	const email = state.auth.user.email;
	const staff = state.settings.staff;
	const user = staff.find(staffPerson => staffPerson.email === email);
	// cut out some code ...
	return {
		// removed some other code ...
		isAdmin: user.role === 'Administrator' ? true : false
	};
}

Then under createPlayer.js I have the line like you mentioned earlier:

player.pending = isAdmin ? false : true;

And the error I get is TypeError: can't assign to property "status" on "<insert long id number here>": not an object and it refers to the file teamData.js Line156:5

@IsaacRaymond
Copy link
Contributor

IsaacRaymond commented Apr 19, 2018

@afranco07 Thanks for the reply! When you've been looking at the app, have you seen any other places where a user's role has been checked? I see that the Administrator, General Manager and Coach roles have been defined in roles.config.js, but I don't see any code that involves their implementation.

I ask because of the following "To Do" in writing out the roles:
Finish writing out roles:

Administrator - has access to all tabs; has access to all permissions

Manager - has access to teams, players, seasons tab; access to all permissions in those tabs

Coach - has access to teams tab; has permissions view team players, input team scores after games

If we're going to check to see if there's an administrator like you've done above, we'll want to stay consistent and use the same format for checking a user's role throughout the app. Sounds like a big task. Should we ask someone at freeCodeCamp if the two of us can go ahead and start working on this implementation?

@makkoli
Copy link
Contributor Author

makkoli commented Apr 20, 2018

@IRGames @afranco07 Hey guys, taking on the role task is a big ask. Significant parts of the app are in need of some refactoring to get it to work. You can join us on our slack at fcc-leagueforgood.slack.com. No one has been really using it, but I'm usually on slack everyday if you want to talk more about this.

@afranco07
Copy link
Contributor

@makkoli how do we sign up for the slack?

@makkoli
Copy link
Contributor Author

makkoli commented Apr 20, 2018

@afranco07 Yeah, I actually forgot I need to invite you to the workspace. If you give me your email I can send you an invite.

@makkoli
Copy link
Contributor Author

makkoli commented Apr 20, 2018

@afranco07 Sent.

@IsaacRaymond
Copy link
Contributor

@makkoli [email protected]
thanks!

@makkoli
Copy link
Contributor Author

makkoli commented Apr 20, 2018

@IRGames sent

@paulywill
Copy link
Member

@makkoli Can this issue be closed?

@afranco07
Copy link
Contributor

@paulywill This issue has been fixed?

@paulywill
Copy link
Member

@afranco07 It's working now. May have been a PR quite some time back.

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

No branches or pull requests

4 participants