Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip alert type postsave during config sync and skip type of alert in block if not present #252

Merged
merged 2 commits into from
May 22, 2023

Conversation

andybroomfield
Copy link
Contributor

@andybroomfield andybroomfield commented May 18, 2023

Fix #251

During config sync operations, if creating a new alert banner type the visibility field will try to be created, even though that is part of config.

This checks if the isSyncing flag is set and skips over the postSave actions as we assume they are part of the config being imported.

Fix #251

During config sync operations, if creating a new alert banner type the
visibility field will try to be created, even though that is
part of config.

This checks if the isSyncing flag is set and skips over the
postSave actions as we assume they are part of the config
being imported.
@Adnan-cds
Copy link
Contributor

Hi Andy,
Thanks for such a prompt response. I have tried the fix on both a fresh site and and existing site. This fix has resolved the issue with the visibility field. But the block config imports are still failing :( This is the final error message I am seeing on a fresh site:

In ConfigImportCommands.php line 324:                                                                                       
                                                                                                                              
    The import failed due to the following reasons:                                                                           
    Unexpected error during import with operation create for block.block.localgov_alert_banner_base: 'type_of_alert' not found                                                                 
    Unexpected error during import with operation create for block.block.localgov_alert_banner_scarfolk: 'type_of_alert' not found

@andybroomfield
Copy link
Contributor Author

Thanks for testing @Adnan-cds
Not sure why the blocks would cause the issue, needs further investiagtion.
Are you installing the theme alongside the alert banner module in the config sync, or just the module on its own (localgov_base / scarfolk already installed)?

@Adnan-cds
Copy link
Contributor

Are you installing the theme alongside the alert banner module in the config sync, or just the module on its own

Just the localgov_alert_banner module and its related configs. The themes got installed earlier as part of the localgov profile during a fresh site install.

@andybroomfield
Copy link
Contributor Author

Just did a quick test and I noticed the type of alert field is installed after the block.

+------------+------------------------------------------------------------------------------+-----------+
|            | field.storage.localgov_alert_banner.visibility                               | Create    |
|            | field.storage.localgov_alert_banner.link                                     | Create    |
|            | localgov_alert_banner.localgov_alert_banner_type.localgov_alert_banner       | Create    |
|            | field.field.localgov_alert_banner.localgov_alert_banner.visibility           | Create    |
|            | field.field.localgov_alert_banner.localgov_alert_banner.link                 | Create    |
|            | block.block.localgov_alert_banner_base                                       | Create    |
|            | block.block.localgov_alert_banner_scarfolk                                   | Create    |
|            | field.storage.localgov_alert_banner.type_of_alert                            | Create    |
|            | field.field.localgov_alert_banner.localgov_alert_banner.type_of_alert        | Create    |

@andybroomfield andybroomfield changed the title Skip alert type postsave during config sync Skip alert type postsave during config sync and skip type of alert in block if not present May 22, 2023
@andybroomfield
Copy link
Contributor Author

@Adnan-cds I've posted an additional fix to try and address this. Its beacuse the type_of_alert field is not present when creating an instance of the alert banner block, as it's not imported yet. Although we need to look at the order, as its possible for users to delete the field (should they be able to?) then I've added a check to the sort in getCurrentBanners so it can be skipped over, alongside a test that confirms the banner block still loads and order is just date changed order.

This should allow your import to work, and then we can consider the issues this has raised.

…ent banners

During import, and in times where the type_of_alert field has been deleted, the
getCurrentBanners method will try to perform an entity query, but it needs to
check the field is present before adding a sort.
@andybroomfield andybroomfield force-pushed the fix/1.x/251-fix-config-sync branch from 7063c6d to da51533 Compare May 22, 2023 11:41
Copy link
Contributor

@Adnan-cds Adnan-cds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me. Many thanks for the prompt fix :)

@andybroomfield andybroomfield merged commit 3077fe8 into 1.x May 22, 2023
@andybroomfield andybroomfield deleted the fix/1.x/251-fix-config-sync branch May 22, 2023 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Config import failure after initial localgov_alert_banner install
2 participants