vault icon indicating copy to clipboard operation
vault copied to clipboard

UI: Ember-data upgrade 5.3.2 prep: use custom service instead of extending ember-data store

Open hellobontempo opened this issue 4 months ago • 3 comments

Description

This PR un-extends the store service and moves lazyPagintedQuery methods to Pagination service to prepare for upgrading to "ember-data": "~5.3.2"


When initially upgrading ember-data and @ember-data/legacy-compat to 5.3.2 we received this error:

router.js:1129 Error while processing route: vault.cluster.index this.store.adapterFor is not a function TypeError: this.store.adapterFor is not a function
    at VersionService.getType (http://localhost:4200/ui/assets/vault.js:64936:41)
    at getType.next (<anonymous>)
  
ember-environment.js:21 Uncaught TypeError: this.store.adapterFor is not a function
    at VersionService.getType (version.js:74:1)
    at getType.next (<anonymous>)
    at GeneratorState.step (generator-state.js:19:53)
    at TaskInstanceExecutor.generatorStep (executor.js:284:42)
    at TaskInstanceExecutor.handleResolvedContinueValue (executor.js:152:27)
    at TaskInstanceExecutor.proceedSync (executor.js:116:12)
    at TaskInstanceExecutor.start (executor.js:54:10)
    at TaskInstance.start (base.js:57:19)
    at refresh.js:27:67
    at Array.forEach (<anonymous>)

Though it's unclear what exactly was causing this error, it was clear the store was failing to compile properly and seemed to trace back to the createCache hook.

image

Deleting our store.js file fixed the build failure and so after some discussion we decided maintaining our custom store extension was become too costly -- every upgrade cycle it required more and more bandaids (https://github.com/hashicorp/vault/pull/25272, https://github.com/hashicorp/vault/pull/22838)

We were just extending the store for lazyPaginatedQuery which manages client-side pagination so we opted to move these methods to a custom Pagination service. Although this functionality would probably be better as a decorator (thank you @Monkeychip for the research and suggestion), that refactor is out of scope for this work. (It requires updating all outdated class syntax to use native javascript classes). For now, moving the methods to a separate service was the most feasible, elegant solution. In https://github.com/hashicorp/vault/pull/28690, addressed some minor cleanup to the store service so updates here would be strictly un-extending the store service and upgrading ember data.

TODO only if you're a HashiCorp employee

  • [ ] Backport Labels: If this PR is in the ENT repo and needs to be backported, backport
    to N, N-1, and N-2, using the backport/ent/x.x.x+ent labels. If this PR is in the CE repo, you should only backport to N, using the backport/x.x.x label, not the enterprise labels.
    • [ ] If this fixes a critical security vulnerability or severity 1 bug, it will also need to be backported to the current LTS versions of Vault. To ensure this, use all available enterprise labels.
  • [ ] ENT Breakage: If this PR either 1) removes a public function OR 2) changes the signature of a public function, even if that change is in a CE file, double check that applying the patch for this PR to the ENT repo and running tests doesn't break any tests. Sometimes ENT only tests rely on public functions in CE files.
  • [ ] Jira: If this change has an associated Jira, it's referenced either in the PR description, commit message, or branch name.
  • [ ] RFC: If this change has an associated RFC, please link it in the description.
  • [ ] ENT PR: If this change has an associated ENT PR, please link it in the description. Also, make sure the changelog is in this PR, not in your ENT PR.

hellobontempo avatar Oct 12 '24 00:10 hellobontempo