Skip to content
This repository was archived by the owner on Jun 11, 2025. It is now read-only.

implementation of external app#323

Merged
nxtcoder17 merged 1 commit into
mainfrom
feat/external-app
May 22, 2024
Merged

implementation of external app#323
nxtcoder17 merged 1 commit into
mainfrom
feat/external-app

Conversation

@nxtcoder19
Copy link
Copy Markdown
Contributor

@nxtcoder19 nxtcoder19 commented May 21, 2024

Resolves #322

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey @nxtcoder19 - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟡 General issues: 3 issues found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

return r.Domain.InterceptApp(newResourceContext(cc, envName), appname, deviceName, intercept, pmappings)
}

// CoreCreateExternalApp is the resolver for the core_createExternalApp field.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

suggestion: Consider adding logging for error cases

Adding logging for error cases within the resolver functions can help with debugging and monitoring. Consider logging the errors before returning them.

Suggested change
// CoreCreateExternalApp is the resolver for the core_createExternalApp field.
func (r *mutationResolver) CoreCreateExternalApp(ctx context.Context, envName string, externalApp entities.ExternalApp) (*entities.ExternalApp, error) {
cc, err := toConsoleContext(ctx)
if err != nil {
log.Printf("Error converting to console context: %v", err)
return nil, err
}

return true, nil
}

// CoreListExternalApps is the resolver for the core_listExternalApps field.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

suggestion: Consider adding logging for error cases

Adding logging for error cases within the resolver functions can help with debugging and monitoring. Consider logging the errors before returning them.

Suggested change
// CoreListExternalApps is the resolver for the core_listExternalApps field.
import "log"
// CoreListExternalApps is the resolver for the core_listExternalApps field.
func (r *queryResolver) CoreListExternalApps(ctx context.Context, envName string, search *model.SearchExternalApps, pq *repos.CursorPagination) (*model.ExternalAppPaginatedRecords, error) {
cc, err := toConsoleContext(ctx)
if err != nil {
log.Printf("Error converting to console context: %v", err)
return nil, err
}

@@ -0,0 +1,52 @@
package entities
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

suggestion: Consider adding validation for ExternalApp fields

Adding validation for the fields in the ExternalApp struct can help ensure data integrity and prevent potential issues.

@nxtcoder17 nxtcoder17 merged commit 157bf0f into main May 22, 2024
@nxtcoder17 nxtcoder17 deleted the feat/external-app branch May 22, 2024 04:49
abdheshnayak pushed a commit that referenced this pull request Nov 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

✨Implement External App api

2 participants