-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
[analyze] Add analyzer interface for Shopify #3226
[analyze] Add analyzer interface for Shopify #3226
Conversation
2bdf69c
to
1b15793
Compare
|
||
resource := &analyzers.Resource{ | ||
Name: info.ShopInfo.Shop.Name, | ||
FullyQualifiedName: info.ShopInfo.Shop.Email, |
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.
It seems plausible that the email can be reused across shops. Maybe we can use the domain instead? There's a lot of information in the shop.json API to choose from.
{ | ||
"Resource":{ | ||
"Name":"My Store", | ||
"FullyQualifiedName":"[email protected]", | ||
"Type":"shop", | ||
"Metadata":{ | ||
"created_at":"2024-08-16T17:16:17+05:00" | ||
}, | ||
"Parent":null | ||
}, | ||
"Permission":{ | ||
"Value":"Read", | ||
"Parent":null | ||
} | ||
}, | ||
{ | ||
"Resource":{ | ||
"Name":"My Store", | ||
"FullyQualifiedName":"[email protected]", | ||
"Type":"shop", | ||
"Metadata":{ | ||
"created_at":"2024-08-16T17:16:17+05:00" | ||
}, | ||
"Parent":null | ||
}, | ||
"Permission":{ | ||
"Value":"Read", | ||
"Parent":null | ||
} | ||
}, |
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.
These bindings look exactly the same. Are we setting the permission correctly?
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.
It's also just "Read" which makes me think we're not using the generated permission values.
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.
thank you for pointing it out. I noted that permissions where being assigned to shop resource instead of categories. Fixed this issue.
da3b527
to
273e8db
Compare
Description:
This PR implements the analyzer interface for shopify.
Checklist:
make test-community
)?make lint
this requires golangci-lint)?