-
Notifications
You must be signed in to change notification settings - Fork 66
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
Fixed build error and AcquireToken function #11
base: master
Are you sure you want to change the base?
Conversation
Hi sureshchahal, thanks for adding this functionality! (Thanks also to alenny for the original code!) I wanted to try out your code so I cloned it from https://github.com/sureshchahal/angular2-adal and then I ran 'npm install' and 'gulp build'. I got errors because there were two TypeScript declaration files for adal.js, so I removed the line in typings.json so I was only left with the one under 'src\adal'. However there were still a number of build errors, as shown below. Were you using a more updated declaration file for adal.js? [16:39:17] Using gulpfile ~\git\sureshchahal\angular2-adal\gulpfile.js |
I have fixed those errors. Also provided AuthHttp class. This will acquire On Mon, Aug 22, 2016 at 2:06 AM, Neil665 [email protected] wrote:
|
Thanks, I've tried it out and it is working well. @alenny are you thinking of merging this pull request into the main library? |
AuthHttp takes RequestOptions in get, post, delete just like underlying Http from angular. Makes a copy of the RequestOptions before passing to Http.
AuthHttp can take RequestOptions in http requests
…n adal config Also fix error if server return 204 status
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for this PR Suresh, I was also making something similar to get the acquire token working , and then your PR saved the day..
i hope you get these changes published soon..
@@ -83,7 +112,7 @@ export class AdalService { | |||
cb(tokenOut); | |||
} | |||
}); | |||
}); | |||
})(); | |||
} | |||
|
|||
public getUser(): Observable<adal.User> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you update the below line
return Observable.bindCallback(function (cb: (u: adal.User) => void) {
to
return Observable.bindCallback((cb: (u: adal.User) => void) => {
as the ts compiler is behaving weirdly with first format and creating a JS of
AdalService.prototype.getUser = function () {
return Observable_1.Observable.bindCallback(function (cb) {
this.adalContext.getUser(function (error, user) {
instead of this
AdalService.prototype.getUser = function () {
var _this = this;
return Observable_1.Observable.bindCallback(function (cb) {
_this.adalContext.getUser(function (error, user) {
which is failing with null reference at this.adalContext.something
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
src/services/authHttp.service.ts
Outdated
,private adalService : AdalService | ||
) { } | ||
|
||
get(url: string, options?: RequestOptionsArgs): Observable<any> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you are making this , it would be really helpful if you can include these methods as well
PATCH
PUT
HEAD
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
src/services/authHttp.service.ts
Outdated
} | ||
|
||
var resource = this.adalService.GetResourceForEndpoint(url); | ||
var authenticatedCall = this.http.request(url, options).map(this.extractData).catch(this.handleError); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather have this assigning statement only if resource is undefined, rather than overriding it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
@alenny Any plans on merging this and releasing , Eagerly waiting for the release. |
Hello kaleemxii- if you can make these changes and submit a change request On Thursday, September 29, 2016, kaleemxii [email protected] wrote:
|
made the changes. please take latest On Thu, Sep 29, 2016 at 4:08 PM, suresh chahal [email protected]
|
Wow that was quick, appreciate it 👍 |
published on NPM: https://www.npmjs.com/package/ng2-adal On Thu, Sep 29, 2016 at 4:56 PM, kaleemxii [email protected] wrote:
|
Thanks @sureshchahal, great work. Thanks also to @alenny for the original code. |
body was ignored in Put, Post and Patch
removed debugger
Don´t log errors to console
Fix Throw error AuthHttp.service
The user object is always being defaulted so if (user) will always be true.
For a full description of the problem and suggested fix see #64
Fix reference to this in getUser error
Eliminate dead code in adal.service
Fix Typescript >= 2.4.1 compilation error described on issue #64
Read me for angular 4
bumped version to 2.0 to track angular 4 version.
Added the breaking change notice and improved formatting
Update readme
Updated README with the new example URL
you get here when calling getUser() before logging in. The adal library then throws 'User information is not available' string
Allow to retrieve login status on adalService asynchronously via observables
this was undefined in getUser error handler
Add angular 5 and replace AuthHttp with new Http Interceptor
Fixed acquireToken and build error