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

[REF] APIv4 - Deprecate and stop using PreSaveSubscriber #22648

Merged
merged 1 commit into from
Feb 1, 2022

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented Jan 27, 2022

Overview

Refactors internal APIv4 code to more reliably handle pre-save processing.

Technical Details

The PreSaveSubscriber was added in the early days of APIv4 to tweak inputs before saving a record, but it was unreliable because:

  • It would run for Create and Update but not Save actions
  • It would run before pseudoconstant suffixes had been resolved
  • It would run before any formatting or internal processing had taken place

Meanwhile, a new pattern of overriding the create/save/update actions and sharing a trait has become more common and while a bit more cumbersome to implement, it does not have the above limitations.

This removes all inheritors of PreSaveSubscriber but keeps it around with a deprecation warning in case any extension in the Universe happens to rely on it.

@civibot
Copy link

civibot bot commented Jan 27, 2022

(Standard links)

@civibot civibot bot added the master label Jan 27, 2022
@colemanw colemanw force-pushed the removeApiv4PreSaveSubscriber branch from dc547d8 to e8562ab Compare January 27, 2022 23:46
The PreSaveSubscriber allowed APIs to tweak inputs before saving a record,
but it was unreliable because:
- It would run for Create and Update but not Save actions
- It would run before pseudoconstant suffixes had been resolved
- It would run before any formatting or internal processing had taken place

Meanwhile, a new pattern of overriding the create/save/update actions and sharing a trait
has become more common and while a bit more cumbersome to implement, it does not share
those limitations.

This removes all uses of PreSaveSubscriber, refactors those bits into Save traits, and
adds a deprecation warning in case any extension in the Universe happens to have implemented it.
@colemanw colemanw force-pushed the removeApiv4PreSaveSubscriber branch from e8562ab to ed3f587 Compare January 28, 2022 01:23
@seamuslee001
Copy link
Contributor

Test Coverage should cover us here and this seems fine to me

@seamuslee001 seamuslee001 merged commit dbfb218 into civicrm:master Feb 1, 2022
@colemanw colemanw deleted the removeApiv4PreSaveSubscriber branch February 1, 2022 03:38
@durian808
Copy link

Should I file a bug report on the above mentioned problem? Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants