Re-add custom flash service so engines can extend cluster's template #19963
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit b04751d.
hellobontempo
changed the title
hellobontempo
changed the title
hellobontempo
commented
Apr 3, 2023
kiannaquach
reviewed
Apr 3, 2023
| @@ -46,7 +46,7 @@ export default class App extends Application { | |||
| }, | |||
| kubernetes: { | |||
| dependencies: { | |||
| services: ['router', 'store', 'secret-mount-path', 'flashMessages'], | |||
| services: ['router', 'store', 'secret-mount-path', 'flash-messages'], | |||
hellobontempo
commented
Apr 3, 2023
There was a problem hiding this comment.
original original file: https://github.com/hashicorp/vault/pull/19925/files#diff-5ebd44ba5909c95c331fd105e463a0332fa06bfc2f1244a4324d2518c29d9e59
(still not using stickyInfo anywhere)
import FlashMessages from 'ember-cli-flash/services/flash-messages';
export default class FlashMessageService extends FlashMessages {
stickyInfo(message: string) {
return this.info(message, {
sticky: true,
priority: 300,
});
}
}
hellobontempo
commented
Apr 3, 2023
| @@ -9,7 +9,7 @@ import { action } from '@ember/object'; | |||
| import { task } from 'ember-concurrency'; | |||
| import { waitFor } from '@ember/test-waiters'; | |||
| import errorMessage from 'vault/utils/error-message'; | |||
| import FlashMessageService from 'ember-cli-flash/services/flash-messages'; | |||
| import FlashMessageService from 'vault/services/flash-messages'; | |||
There was a problem hiding this comment.
anytime we see FlashMessageService we know it refers to the custom service file
hellobontempo
changed the title
hellobontempo
commented
Apr 3, 2023
| @@ -16,7 +16,7 @@ export default class KubernetesEngine extends Engine { | |||
| modulePrefix = modulePrefix; | |||
| Resolver = Resolver; | |||
| dependencies = { | |||
| services: ['router', 'store', 'secret-mount-path', 'flashMessages'], | |||
| services: ['router', 'store', 'secret-mount-path', 'flash-messages'], | |||
There was a problem hiding this comment.
opted to make syntax the same across the app
kiannaquach
approved these changes
Apr 3, 2023
hellobontempo
deleted the
revert-19925/ui/remove-flashmessage-custom-service
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Our test suite hasn't been up and running on github the last week which caused some breaking changes to be merged accidentally.
#19925 was opened to resolve the flash message service not calling our custom
stickyInfomethod (more details in this comment)However, this PR introduced new errors because we missed removing the
flash-messageservice from the individual engine dependancies and updating an import file path that was merged in from a separate branch.To address these I opened #19950 but then discovered none of the flash messages in the engines were rendering because each ember engine needs its own
<FlashMessage>template to render since it no longer had access to the FlashMessage template in cluster.hbs (see closing comment here)This PR keeps the removal of
stickyInfofrom #19925 (because I couldn't reproduce the original error and didn't want to trust renaming resolved it) but will open a draft with the rest of the changes if we decided to addember-cli-flashas a dependency to each engine.Aforementioned draft: #19969