Skip to content

Conversation

@a7maad-ayman
Copy link
Collaborator

@a7maad-ayman a7maad-ayman commented Jul 16, 2023

Proposed Changes

implement login and register functionality using Firebase without adding the SDK, you can make HTTP requests directly to Firebase's Authentication REST API.

Related Issues

HL-76

List any related issues or pull requests.
HL-75
Networking

Provide any additional information or context that may be relevant.

Screenshot

Required for any UI changes

Checklist

  • I have tested these changes locally.
  • I have added appropriate documentation or updated existing documentation.
  • I have added appropriate test coverage or updated existing test coverage.
  • I have updated the changelog (if applicable).
  • I have followed the project's code style and formatting guidelines.
  • I have reviewed and adhered to the project's contributing guidelines.

Comment on lines 24 to 25
public var path: String { "AIzaSyB0UczrurqM1STyI8tvx4QZVTyQVw4UJ7Q" }
public var method: String = "POST"
Copy link
Collaborator Author

@a7maad-ayman a7maad-ayman Jul 16, 2023

Choose a reason for hiding this comment

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

is it right to pass the apiKey for the path
or should it be act like a query parameter
@ahmdmhasn

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

And I have actually written the Unit tests last week. I've been waiting for your approval 😅

Copy link
Contributor

Choose a reason for hiding this comment

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

Please unit test this clss to make sure the parameters and other values are sent as expected

@a7maad-ayman a7maad-ayman changed the title feat: create FirebaseLoginRequest 🍕 feat:[HL-76] create FirebaseLoginRequest 🍕 Jul 17, 2023
Copy link
Contributor

@ahmdmhasn ahmdmhasn left a comment

Choose a reason for hiding this comment

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

Great effort, just a few steps...


public var baseUrl: URL { Constants.firebaseAuth }
public var path: String { "AIzaSyB0UczrurqM1STyI8tvx4QZVTyQVw4UJ7Q" }
public var method: String = "POST"
Copy link
Contributor

Choose a reason for hiding this comment

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

You either use

Suggested change
public var method: String = "POST"
public var method: String { "POST" }

or

Suggested change
public var method: String = "POST"
public let method: String = "POST"

Comment on lines 24 to 25
public var path: String { "AIzaSyB0UczrurqM1STyI8tvx4QZVTyQVw4UJ7Q" }
public var method: String = "POST"
Copy link
Contributor

Choose a reason for hiding this comment

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

Please unit test this clss to make sure the parameters and other values are sent as expected

}

public var baseUrl: URL { Constants.firebaseAuth }
public var path: String { "AIzaSyB0UczrurqM1STyI8tvx4QZVTyQVw4UJ7Q" }
Copy link
Contributor

Choose a reason for hiding this comment

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

This is no the path... pease check the register request as an example

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i get a little confused about that

Comment on lines +4 to +12
public struct FirebaseLoginResponse: Codable {
let kind, localID, email, displayName: String
let idToken: String
let registered: Bool
let refreshToken, expiresIn: String
}

// MARK: - FirebaseLoginRequest
public struct FirebaseLoginRequest: RequestType {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
public struct FirebaseLoginResponse: Codable {
let kind, localID, email, displayName: String
let idToken: String
let registered: Bool
let refreshToken, expiresIn: String
}
// MARK: - FirebaseLoginRequest
public struct FirebaseLoginRequest: RequestType {
public struct LoginResponse: Codable {
let email, displayName: String
let idToken: String
let registered: Bool
let refreshToken, expiresIn: String
}
// MARK: - FirebaseLoginRequest
public struct LoginRequest: RequestType {

let kind, localID, email, displayName: String
let idToken: String
let registered: Bool
let refreshToken, expiresIn: String
Copy link
Contributor

Choose a reason for hiding this comment

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

expiresIn should be converted to Date

@a7maad-ayman a7maad-ayman requested a review from ahmdmhasn July 21, 2023 11:32
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.

4 participants