UI: Move useOpenApi and getHelpUrl methods to util#27764
Conversation
github-actions
bot
added
the
hashicorp-contributed-pr
|
Build Results: |
|
CI Results: |
hellobontempo
left a comment
hellobontempo
left a comment
There was a problem hiding this comment.
Great cleanup work to move us towards more ember-data happy models! Just a couple non-blocking comments
| }; | ||
|
|
||
| export function getHelpUrlForModel(modelType: string, backend: string) { | ||
| const urlFn = OPENAPI_POWERED_MODELS[modelType as keyof typeof OPENAPI_POWERED_MODELS] as ( |
There was a problem hiding this comment.
do we need both keyof and typeof here?
There was a problem hiding this comment.
Yup, because OPENAPI_POWERED_MODELS is a const not an interface, so we need typeof so typescript is happy
| const model = this.store.createRecord(modelName, {}); | ||
| // Generated items don't have this method -- helpUrl is calculated in path-help.js line 101 | ||
| const helpUrl = model.getHelpUrl(this.mount); | ||
| // Generated items don't have helpUrl method -- helpUrl is calculated in path-help.js line 101 |
There was a problem hiding this comment.
thank you for this comment! do we have test coverage that assures removing the || newModel.proto().getHelpUrl(backend); of the conditional in path-help is alright?
From what I gather it seems like all non-generated items have paths defined in the new helper, getHelpUrlForModel and any generated items should otherwise follow the pattern of /v1/${apiPath}${path.slice(1)}?help=true - just wanted to double check!
There was a problem hiding this comment.
Good question! I removed the || because the first statement would never be falsey.
// original
helpUrl = `/v1/${apiPath}${path.slice(1)}?help=true` || newModel.proto().getHelpUrl(backend);
even if apiPath and path.slice(1) resolve to empty strings, the first part will resolve at minimum to /v1/?help=true.
Otherwise your comment is true -- if we have the base model for it, the helpUrl should be defined in getHelpUrlForModel. If it's generated (like for userpass users) then it follows the pattern in line 101 of path-help service
There was a problem hiding this comment.
Also, this is a skipped test -- I just didn't want any instances of getHelpUrl laying around so I could be sure I got them all 😂
There was a problem hiding this comment.
great thinking! nice to keep up with tech debt while we have context. awesome! that all makes sense, thanks for explaining.
* Add map between model types and helpUrls, update tests * replace modelProto.getHelpUrl with new helper util * Remove all useOpenApi and getHelpUrl instances from models * Add missing auth config model type
* Add map between model types and helpUrls, update tests * replace modelProto.getHelpUrl with new helper util * Remove all useOpenApi and getHelpUrl instances from models * Add missing auth config model type
2 participants
Description
This is a cleanup item to remove
useOpenApiandgetHelpUrlfrom the models (which don't have anything to do with the shape of the data) and into a util that thepath-helpservice can then utilize to get the appropriate data based on the model type and backend path. There should be no user-facing changes with this PR, but is foundation work for eventually upgrading to Ember Data 5