Skip to content
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

NOISSUE - Add Domains Service #8

Merged
merged 8 commits into from
Apr 4, 2024
Merged

Conversation

Musilah
Copy link
Contributor

@Musilah Musilah commented Mar 26, 2024

What does this do?

This adds the Domains Service to the sdk.

Which issue(s) does this PR fix/relate to?

Resolves #13

List any changes that modify/break current functionality

Have you included tests for your changes?

Did you document any new/modified functionality?

Notes

src/domains.ts Outdated
this.domainError = new Errors()
}

public async CreateDomain (domain: Domain, token: string): Promise<Domain | undefined> {
Copy link
Contributor

Choose a reason for hiding this comment

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

we should not provision for a return of undefined assert the type to be Domain only. We should only either return domain or an error, nothing else.

Apply this to the others as well

src/domains.ts Outdated
}
}

public async RemoveUserfromDomain (domainID: string, userIDs: string[], relation: string, token: string): Promise<any> {
Copy link
Contributor

Choose a reason for hiding this comment

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

For this you can return the statuscode

src/domains.ts Outdated
}
}

public async AddUsertoDomain (domainID: string, userIDs: string[], relation: string, token: string): Promise<string> {
Copy link
Contributor

Choose a reason for hiding this comment

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

same for this. return statusCode, or you can create a return interface, that will return the status code and the message.

src/domains.ts Outdated
limit: number
}

interface DomainsInterface {
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
interface DomainsInterface {
interface DomainsPage {

@Musilah Musilah force-pushed the domainsnew branch 2 times, most recently from dc61969 to 7a4787a Compare April 3, 2024 11:51
src/defs.ts Outdated
name?: string
id?: string
alias?: string
email?: string
Copy link
Contributor

Choose a reason for hiding this comment

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

domain does not have param email. The create domain struct looks like so:

type Domain struct {
	ID          string    `json:"id,omitempty"`
	Name        string    `json:"name,omitempty"`
	Metadata    Metadata  `json:"metadata,omitempty"`
	Tags        []string  `json:"tags,omitempty"`
	Alias       string    `json:"alias,omitempty"`
	Status      string    `json:"status,omitempty"`
	Permission  string    `json:"permission,omitempty"`
	CreatedBy   string    `json:"created_by,omitempty"`
	CreatedAt   time.Time `json:"created_at,omitempty"`
	UpdatedBy   string    `json:"updated_by,omitempty"`
	UpdatedAt   time.Time `json:"updated_at,omitempty"`
	Permissions []string  `json:"permissions,omitempty"`
}

src/domains.ts Outdated
}
}

public async EnableDomain (domainID: string, token: string): Promise<Domain> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Enable and disable domain do not return the domain. Just an status code and/or error incase of an error

src/domains.ts Show resolved Hide resolved
Copy link
Contributor

@ianmuchyri ianmuchyri left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@dborovcanin dborovcanin left a comment

Choose a reason for hiding this comment

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

We are handling only successful cases. We should also handle at least NotFound, BadRequest, Redirects, Forbidden/Unauthorized cases, as well. Also,
constructs such as

catch (error) {
      throw error
}

do not make much sense.

@dborovcanin dborovcanin merged commit 6e937e0 into absmach:main Apr 4, 2024
1 check passed
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.

Update JS SDK for Domains
3 participants