Skip to content
This repository has been archived by the owner on Feb 23, 2019. It is now read-only.

Use Agent for admin authentication handling instead of custom handling on Fixtures #212

Merged
merged 1 commit into from
Sep 21, 2017
Merged

Use Agent for admin authentication handling instead of custom handling on Fixtures #212

merged 1 commit into from
Sep 21, 2017

Conversation

jcsmorais
Copy link
Contributor

@jcsmorais jcsmorais commented Sep 19, 2017

Some users are currently experiencing intermittent failures with the following error: Cannot read property '_module' of undefined.

That seems to be related with Fixtures having its own way of handling user authentication for admin users, which might lead into conflicts and unexpected errors if admin users try to log in twice, where the second login ends up invalidating the refresh token from the first login.

This PR is replacing the custom authentication handling for admin users on Fixtures side and making use of the Agent abstraction instead.

This is also related and should fix what has been reported on: #145

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 95.588% when pulling e43e801 on jcsmorais:replace-fixtures-admin-handling into 0a7363a on sugarcrm:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 95.588% when pulling e43e801 on jcsmorais:replace-fixtures-admin-handling into 0a7363a on sugarcrm:master.

});
// Use chakram.post to bulk create the record(s).
bulkRecordCreateDef.requests.push(request);
});
Copy link
Contributor

Choose a reason for hiding this comment

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

The block of code from line 319 ~ 334 is the same as the original, except for indentation. This make this commit change more code than it needed (functionally).

index.js Outdated
let url = utils.constructUrl(VERSION, left._module, left.id, 'link');
let params = {headers: this._headers};
let url = [left._module, left.id, 'link'].join('/');
let params = { };
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove space between an empty object? In fact, the params argument is optional for the "post" per the doc of Chakram, and we don't modify it anywhere. @rhoggSugarcrm said we are aware of this.

@coveralls
Copy link

coveralls commented Sep 20, 2017

Coverage Status

Coverage decreased (-0.3%) to 95.549% when pulling e060665 on jcsmorais:replace-fixtures-admin-handling into 0a7363a on sugarcrm:master.

@coveralls
Copy link

coveralls commented Sep 20, 2017

Coverage Status

Coverage decreased (-0.3%) to 95.549% when pulling ea114ab on jcsmorais:replace-fixtures-admin-handling into 0a7363a on sugarcrm:master.

@jcsmorais jcsmorais changed the title Replace fixtures custom admin handling by Agent.as(Agent.ADMIN) Use Agent for admin authentication handling instead of custom handling on Fixtures Sep 20, 2017
@coveralls
Copy link

coveralls commented Sep 20, 2017

Coverage Status

Coverage decreased (-0.3%) to 95.549% when pulling e99e849 on jcsmorais:replace-fixtures-admin-handling into 0a7363a on sugarcrm:master.

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

Successfully merging this pull request may close these issues.

None yet

4 participants