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

Do not crash the whole SearchKit UI if one entity fails #25433

Merged
merged 1 commit into from
Jan 31, 2023

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Do not crash the whole SearchKit UI if one entity fails

To replicate

  • enable civiimport
  • start an import of some sort
  • go into the database & delete the temp table
  • try to load search kit
  • it doesn't load...

Before

If one entity is broken the entire SearchKit UI is broken. This can happen when civiimport is enabled and an import temp table has been dropped but Managed.reconcile has not yet occurred. I've done a bit of work on trying to prevent this but still see it sometimes & of course a DBA could do it. In addition I've seen it happen with various extension load order issues

After

The UI still loads

Technical Details

The class that is being touched is wholly within SearchKit and is a general 'browse' page - I don't think it makes sense to totally fail here when there is a problem

Comments

@colemanw

@civibot
Copy link

civibot bot commented Jan 26, 2023

(Standard links)

@civibot civibot bot added the master label Jan 26, 2023
@colemanw
Copy link
Member

I have to say it's a pretty weird situation to contemplate. So Entity::get sometimes returns the name of an entity which doesn't actually exist?

@colemanw colemanw merged commit be45150 into civicrm:master Jan 31, 2023
@eileenmcnaughton eileenmcnaughton deleted the sk_admin branch January 31, 2023 20:37
@eileenmcnaughton
Copy link
Contributor Author

@colemanw yeah - due to load order issues - not that rare from my experience - ie working in a dev environment with lots of custom extensions

wmfgerrit pushed a commit to wikimedia/wikimedia-fundraising-crm that referenced this pull request Jan 31, 2023
https: //github.com/civicrm/civicrm-core/pull/25433
civicrm/civicrm-core#25482

Bug: T303986
Change-Id: Iafbe043e5103bf7582805e74e987bc550c56443b
wmfgerrit referenced this pull request in wikimedia/wikimedia-fundraising-crm Feb 16, 2023
Note we have some recent patches that are included. These are mostly ones merged to
civicrm-core since forking for 5.58. One is in 5.58 now so is listed for
completeness & 2 minor ones are not yet merged upstream

|pr|status|notes|
|[25565](https://github.com/civicrm/civicrm-core/pull/25565)|merged to master(5.60)|Supports T327963 - activity types for thank you|
|[25516](https://github.com/civicrm/civicrm-core/pull/25516)|merged to master(5.60)|Helps import - filter imported searches|
|[25527](https://github.com/civicrm/civicrm-core/pull/25527)|not yet merged| but performance fix for dedupe searches (the name & address ones Sandra is doing)
|[25433](https://github.com/civicrm/civicrm-core/pull/25433)|merged to rc (5.59)|fix for search kit display|
|[25482](https://github.com/civicrm/civicrm-core/pull/25482)|Already in 5.58.1|Fix for search kit display|
|[25418](https://github.com/civicrm/civicrm-core/pull/25418)|Merged to rc (5.59)|Permanent imap timout fix|
|[25226](https://github.com/civicrm/civicrm-core/pull/25226)|Merged to rc (5.59)|ReportInstance api - in our install|
|[25568](https://github.com/civicrm/civicrm-core/pull/25568)|not yet merged|Helps with new import code, reduces chance of crash|
|[24515](https://github.com/civicrm/civicrm-core/pull/25415)|Merged to rc (5.59)| Fix to stop CI from breaking, replaces our hack|

List of cherry picked commits from the above

18d82af27b9 (HEAD -> master) Filter expired searches from search kit results
a75b98cb47a Add test to is_current on SavedSearch, fix
e4d69230ad1 Increase timeout on imap
89ab855619f Do not crash the whole SearchKit UI if one entity fails
0142e5dd18c Fix performance on deciding which tables to query
cd710af9733 dev/core#4117 Add is_current to UserJob, Search
2fa17fb88ae Make activity_type_id available to links, test
2d2bd93cda8 (dev/core#4088) ClassScanner - Move unit-test registration
e3e669d4556 Add Report Instance apiv4

Bug: T329681
Change-Id: I839d1bbfabfe3558094f7445a4281f237b609fed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants