Remove 1 banner limitation and add the unpublish others as an option #102
Conversation
|
Drafiting this as a PR so people can check work to date. |
andybroomfield
force-pushed
the
feature/99-unpublish-others-on-confirmation
branch
from
e25dbc5
to
2710c1a
Compare
|
This should be mostly done now, but need to get the test working before merging. |
andybroomfield
force-pushed
the
feature/99-unpublish-others-on-confirmation
branch
from
3774a87
to
b031d08
Compare
…nfirmation Ref #99 WIP: Initial add the unpublish checkbox and allow more than one banner to be live at a time. If the checkbox is ticked, unpublish all other banners.
- Removes the existing check one banner test (as now more than one banner) - Adds extra assertions to the confirmation form test by setting the value of the unpublish others checkbox and verifing if an alternative banner is published or not. (This relies on loading the caniocal banner and checking for the respective Put banner live / Remove banner link).
- Update the entity type defintion - Provide update hook to update the entity - Add get and set token methods to the entity class - Update the js so each banner can be dismissed seperatly
- Updates the JS test to handle precense of multiple banners - Test dismissing each in turn and verified only that banner remains dismissed
- Since the state is now part of the entity, remove the state service
- Goes by the type of alert (Death, Major, Minor, Announcement -- Depending on how the field type_of_alert is set up) - Then by the date changed (most recent first). Note: if the type is missing, then banners with a type get displayed first.
andybroomfield
force-pushed
the
feature/99-unpublish-others-on-confirmation
branch
from
b031d08
to
c6a08e8
Compare
|
This is ready for review now. |
andybroomfield
force-pushed
the
feature/99-unpublish-others-on-confirmation
branch
from
9d89d5d
to
7108d05
Compare
This is becuase , gets url encoded.
|
Thanks @andybroomfield - just testing now. |
|
I've tested this locally on a fresh install of LocalGov Drupal and on the Lambeth site, both appear to work as expected. I can set multiple banners to be live at the same time, and unselect the box to unpublish other banners. Works for me. @ekes want to take a look at the code? |
There was a problem hiding this comment.
Looks good to me!
Tested locally on fresh install of LocalGov Drupal and on a copy of the Lambeth site.
I've taken a cursory look at the code and can't see anything that jumps out.
Tests and functionality work for me so happy to approve.
Thanks @andybroomfield !
|
It would be great to get this one merged. Are you waiting on more reviewing @andybroomfield ? |
|
@finnlewis if your happy I will merge this tomorrow and prep a release. |
Fix #99.
This removes the single alert banner limitation, and replaces it with a checkbox on the alert banner confirmation page to unpublish others.
This also updates the banner order to go by (if the type_of_alert) field is present, by Death of notable person, major, minor, annoucement. Then by date updated.
Move the dismiss token from state api and onto the entity (Note this will require running database updates to update the entity).
Updates the tests to account for new banner behaviour.