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

Add deprecation warnings around code that seems ripe for removal #13789

Closed
wants to merge 1 commit into from

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Add deprecation warnings to code that seems obsolete

Before

Code not marked as obsolete

After

Code marked

Technical Details

This just seems dead...

Comments

@civibot
Copy link

civibot bot commented Mar 8, 2019

(Standard links)

@civibot civibot bot added the master label Mar 8, 2019
@@ -102,6 +102,9 @@ public static function preProcessCommon(&$form) {
$form->_contactTypes = array();

$useTable = (CRM_Utils_System::getClassName($form->controller->getStateMachine()) == 'CRM_Export_StateMachine_Standalone');
if ($useTable) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The real pain point here is the extra code that exists to support this

@demeritcowboy
Copy link
Contributor

It's used in the civicase extension, although I'm not clear how you actually get to it via UI.
https://github.com/civicrm/org.civicrm.civicase/blob/master/ang/civicase/Actions.js#L224
Also https://issues.civicrm.org/jira/browse/CRM-21406

@eileenmcnaughton
Copy link
Contributor Author

@demeritcowboy maybe that's a reason to merge it! There is a lot of code that supports class that has tech debt & is untested when it changes so if we add the deprecations it will either let us know if it's being used or we can remove over time

@colemanw it seems you actually added this - if you look at CRM_Contact_Form_Task:: preProcessCommon it's full of stuff that makes that complicated just to support something that that is not in core & is perhaps not accessible from CiviCase anymore

@colemanw
Copy link
Member

colemanw commented Apr 4, 2019

Yes that was mine, see #11254. It's pretty recent work and is used in the new CiviCase extension. I agree that it could use unit tests but it's not deprecated.

@colemanw colemanw closed this Apr 4, 2019
@eileenmcnaughton
Copy link
Contributor Author

@colemanw does it allow us to bypass security - I see it supports exporting random entities?

From my POV the $useTable stuff in core is a problem IMHO - it's shitty handling in that core for stuff that isn't in core

@colemanw
Copy link
Member

colemanw commented Apr 4, 2019

@eileenmcnaughton eileenmcnaughton deleted the deprecate branch April 4, 2019 01:22
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.

3 participants