add templates for a few magic links #69
Conversation
| @@ -287,7 +291,7 @@ class ARN { | |||
| }, | |||
| "codebuild": { // AWS CodeBuild | |||
| "build": null, | |||
| "project": null, | |||
| "project": () => `https://${this.region}.${this.console}/codesuite/codebuild/projects/${this.resource}`, | |||
There was a problem hiding this comment.
Can you add unit tests for all the resource types that you are adding support for?
It's easy to do, existing tests are here: https://github.com/link2aws/link2aws.github.io/blob/master/testcases/aws.json
| "": null, | ||
| "": () => { | ||
| const parts = this.resource.split('/'); | ||
| const myid = parts[2]; |
There was a problem hiding this comment.
Why myid? This could be just id, or could be more specific (maybe api_id). But my is just visual clutter that doesn't convey information and should be removed.
|
|
||
| // Running as command line script? (not in browser, and not as library) | ||
| /* istanbul ignore if */ | ||
| if (typeof (require) !== 'undefined' && require.main === module) { |
There was a problem hiding this comment.
Why are you removing all this code?
| @@ -164,7 +164,7 @@ class ARN { | |||
| "analyzer": () => `https://${this.region}.${this.console}/access-analyzer/home?region=${this.region}#/analyzer/${this.resource}`, | |||
| }, | |||
| "acm": { // AWS Certificate Manager | |||
| "certificate": () => `https://${this.console}/acm/home?region=${this.region}#/?id=${this.resource}`, | |||
| "certificate": () => `https://${this.console}/acm/home?region=${this.region}#/certificates/${this.resource}`, | |||
There was a problem hiding this comment.
Thanks. I have verified that this change is correct.
However, you also need to update the unit tests. Currently they are failing due to this change. Search for arn:aws:acm:us-east-1:123456789012:certificate/1f6ee793-4064-4a10-9567-f03875640b35 in https://github.com/link2aws/link2aws.github.io/blob/master/testcases/aws.json - the console URL there is currently outdated.
| "": null, | ||
| "": () => { | ||
| const parts = this.resource.split('/'); | ||
| const myid = parts[2]; |
There was a problem hiding this comment.
Can you fix all the whitespace errors?
- There is trailing whitespace at the end of the
const myid = ...line. Please remove it. - The
returnline is indented with a mix of tabs and spaces. It should be spaces only. - The closing
}has the wrong indent level. It should be indented one level less.
| @@ -182,7 +182,11 @@ class ARN { | |||
| }, | |||
| }, | |||
| "apigateway": { // Manage Amazon API Gateway | |||
| "": null, | |||
| "": () => { | |||
| const parts = this.resource.split('/'); | |||
There was a problem hiding this comment.
I understand why you did this, but this is the wrong solution. Actually what has happened is that you uncovered a bug (very nice! Thank you!). The ARN was parsed via the // ...:resource-id code branch in the constructor() function, but should have been parsed via the // ...:resource-type/resource-id (resource-id can contain slashes!) code branch. Instead of splitting here, can you just change > 0 to >= 0 in the the existing code - from this:
// ...:resource-type/resource-id (resource-id can contain slashes!)
else if (typeof (tokens[5]) != 'undefined' && tokens[5].indexOf('/') > 0) {
to this:
// ...:resource-type/resource-id (resource-id can contain slashes!)
else if (typeof (tokens[5]) != 'undefined' && tokens[5].indexOf('/') >= 0) {
Then you can simply use the "": () => `...`, pattern here.
I have verified that all existing tests continue to pass with that change. So it really is a case that wasn't exercised before.
adding a few templates for links
acm cert
apigateway
codebuild project
eip-allocation
ecr repository
ecs environment
es domain
glue job
rds snapshot