Weather Story WFO Publish Confirmation #1392
Merged
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
greg-does-weather
requested changes
Jun 27, 2024
Comment on lines
109
to
112
| // First, archive any Weather Story nodes that are already | ||
| // published if they have the same WFO as was submitted | ||
| $wfo_id = $formState->getValue("field_wfo")[0]["target_id"]; | ||
| weathergov_unpublish_published_weather_stories($wfo_id); |
There was a problem hiding this comment.
I think there needs to be a check around this for the node type. Otherwise, publishing a new node of any type that happens to have a WFO field will unpublish weather stories for that WFO.
To reproduce:
- create a weather story for a WFO
- publish it
- create a WFO promo for the same WFO
- publish it
There was a problem hiding this comment.
Whoa, oof. Good catch
There was a problem hiding this comment.
Should be fixed now in 7cd1aba thanks
Comment on lines
+81
to
+85
| // The data attribute will reference the id | ||
| // of a target button to programmatically click | ||
| // on the page's actual form. | ||
| // If there is no target, the modal will simply close | ||
| const targetButton = document.getElementById( |
There was a problem hiding this comment.
This is clever.
As pointed out by Greg, publishing any other type of content node was unpublishing all Weather Story nodes because we were not checking the node type during the publish hook. This commit adds a check.
eric-gade
force-pushed
the
eg-1269-basic-js-ws-confirmation
branch
from
9b8a534
to
690e6ba
Compare
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.
What does this PR do? 🛠️
After eons, this PR addresses #1269. The gist is that when creating a new Weather Story and entering a linked WFO, authors should see a dialog of some sort informing them that there is another Weather Story already published with the same WFO. The dialog will have options to save a draft, cancel, or "replace" -- meaning unpublish the other Weather Story first, then publish the new one.
What does the reviewer need to know? 🤔
Hey, isn't there a way to already do this?
No.
I spent a significant amount of time trying to get this "confirmation, with original form data" concept to work via various Drupal APIs and patterns. None of them worked.
After days of poking at it, I threw my hands up and decided to go with Javascript and some basic hook/template alterations.
Ok, sure, but how does it work?
We add a custom JS file as a theme library called
weather-story-modal-dialogwhich handles the logic of an otherwise inert<dialog>element that is now present in the template for adding weather stories.In the admin theme file, we add a few helper methods and hooks that will load this library.
Additionally, if the form is a Weather Story create form, the hook will query the system for all weather stories that are currently published, passing information about all of them -- including and especially information about their linked WFO Terms -- to the template file as a JSON object tucked away in a script tag.
If the user decides to publish anyway, all existing published Weather Stories with the same WFO will be archived. This happens by default in the publish action now, and is not unique to the dialog.
Boring! How can I just try it out?
Here are some steps to follow to try it out locally:
zapyour containersScreenshots (if appropriate): 📸
Visual summary of "replacing" with a new weather story for Nashville