MosaicGroups / mosaic-groups

A Mosaic groups application
GNU General Public License v3.0
6 stars 2 forks source link

Error appearing when leaders try to add their groups #214

Closed amandawmosaic closed 7 years ago

amandawmosaic commented 7 years ago
screen shot 2017-08-02 at 9 32 01 am
amandawmosaic commented 7 years ago

Today Ricky Hartford tried to enter his group's information and received the same error pictured above. He tried to enter his group at around 1pm

BKreisel commented 7 years ago
Aug 09 10:04:57 mosaicgroups heroku/router:  at=info method=POST path="/api/groups" host=mosaicgroups.org request_id=71919b08-e93d-426c-8fe3-7f0bb17a1333 fwd="136.160.248.143" dyno=web.1 connect=1ms service=68ms status=200 bytes=1368 protocol=https 

Aug 09 10:04:57 mosaicgroups app/web.1: Request: [Wed, 09 Aug 2017 17:04:57 GMT](::ffff:10.99.212.2): "POST /api/groups HTTP/1.1" 200 1140 

Aug 09 10:05:00 mosaicgroups app/web.1: log: { message: '250 2.0.0 OK 1502298299 f56sm2456323qta.79 - gsmtp', 

Aug 09 10:05:00 mosaicgroups app/web.1:   messageId: '1881f45c56e789441eed06bb2a7e2b@5d17338d-c223-4bd4-a78d-caf65c509d43' } (in emailer.js:141) 

The only logs I see for today on the server itself (for the group creation endpoint) appear to correspond to a successful group creation at what I believe is 10:04AM GMT.

It looks like the error is generated in groups.js

export const addGroup = (group) => (dispatch, getState) => {
    group.members = [];
    dispatch({
        type: ADD_GROUP,
        group
    });
    return request.post('/api/groups')
        .send(group)
        .then(response => {
            toastr.success('Success', `You have successfully added ${group.title}`);
            dispatch(push('/'));
        })
        .catch(err => {
            toastr.error('Error', `There was an error adding ${group.title}, Please send this error to the group coordinator:  ${err}`);
        });

};

I'm not seeing an HTTP 4XX or 5XXerrors in the logs for today either.

The specific error above TypeError: e.leaders is undefined looks like an exception thrown from the client side javascript. I'm a bit rusty on React/Redux so someone else might want to chime in but it looks like group validation is failing before even sending off the request to the server.

jeffklassen commented 7 years ago

@amandawmosaic thanks for the screenshot!!

@BKreisel great work running this down -- let me know what you find!

mnmercer commented 7 years ago

I think I've found the location in the JavaScript that's causing this issue. It appears to be the utility method userIsLeaderOfGroup (copied below). It seems that this method is called at some point during the process with a group that does not have a leaders property. I am able to recreate that situation on my local build of master by creating a group without a leader selected, however the error does not appear. In the logs, it seems that when we update the list of groups after submission of the form, we simply push whatever data we have, which in this situation doesn't have the leaders property. At some point, this incomplete group object is replaced by one with a leaders property set.

I will see if I can ensure that the group always has default values for the major properties and I think that should solve the issue. However, it will be hard to confirm the fix since we can't reproduce the error locally.

const userIsLeaderOfGroup = (group, identity) => {
    // filter leaders to see if the logged in user is a leader of the group
    return group.leaders
        .filter(leader => identity._id === leader._id)
        .length > 0;
};
jeffklassen commented 7 years ago

I added another Pull Request that addresses this in a slightly different way and cleans up a few other bugs.

To reproduce the error locally, all you need to do is login as a non-admin user to the site.

It looks like for non-admin users, the default set of checkboxes in the LeaderCheckboxes component were not being set correctly. Additionally, when the default leader (the authenticated user) checkbox was checked on initialization, this was not triggering an onUpdate event in the DOM (Document Object Model) so the application state was never being notified of this setting.

I've changed the LeaderCheckboxes component to correctly disable checkboxes based on the authenticated user's role and if no leader value is present or the leader value is an empty array, I default to the authenticated user. This behavior was already occurring on the server, but since we've added additional client-side verification, the server was never seeing the group submission.

I've also fixed some additional bugs surrounding non-admin authenticated users manipulating their groups.

I think we should go with PR #217 instead of PR #216 as #217 is a more complete fix.

jeffklassen commented 7 years ago

@amandawmosaic This has been fixed for you!