CRM-16865: Mass Dedupe Workflow Improvements #6432
Conversation
| * @return string | ||
| * | ||
| */ | ||
| public static function encodeDataTable($params, $iTotal, $iFilteredTotal, $selectorElements) { |
There was a problem hiding this comment.
Does this precede the datatable updates @colemanw did?
There was a problem hiding this comment.
@deepak-srivastava this function does not look necessary.
It seems to be duplicating the legacy code in encodeDataTableSelector which is deprecated. In 4.7+ we simply use CRM_Utils_JSON::output() to output json.
|
Just a quick note - we've been doing a good job of getting comment block-consistency in core (ie. working towards all functions havin a comment block) - I see there are quite a few new functions without them - could you add them? The full phpcs standard is a block for all functions, there should be a one-line description ending in a full stop and if appropriate a blank line before further comments, every param should have a type (& preferably a description - but we've made less progress on this). |
|
The datatable implementation here seems to be modeled on legacy code. In 4.7+ we do not invoke datatables like Please see the commits in https://issues.civicrm.org/jira/browse/CRM-16353 for example ad280fb |
|
@deepak-srivastava looks like PR has gone stale, can you please rebase |
…7e53e0 (re-order columns and disable sorting for icon column)
… conflict labels & values in conflict. 3. layout for conflict column - new line after every conflict info. 4. 'vs' formatting. 5. Add permission for forced merges. 6. Fix for: 'One of parameters (value: null) is not of the type Money/Timestamp' errors during batch merges. 7. allow hooks to decide if to skip merges even in aggressive mode. 8. for conflicts screen provide option for safe merges as well
…e searches for datatables
deepak-srivastava
force-pushed
the
dedupe-workflow-47
branch
from
d34dbe1
to
183ec33
Compare
|
@colemanw Thanks. Think i've got it mostly working with crmAjaxTable plugin - deepak-srivastava@f2515ec Not sure about these two points (and anything else that you might see?) :
Any ideas / pointers ? |
|
Thanks for getting rid of that extra encodeDataTable function. Much cleaner. Will comment inline for your other questions. |
| {data: "dst_postcode"}, | ||
| { | ||
| data: "conflicts", | ||
| className: "crm-pair-conflict" |
There was a problem hiding this comment.
You can do this simply by adding this class to the header e.g. <td class="crm-pair-conflicts"> and it will be copied to each cell.
|
@colemanw thanks. rowCallback worked. className didn't. className remains in header (th), doesn't get copied for td. Any ideas? |
|
Sorry, should be "cell-class". Here's an example: On 08/12/2015 04:21 PM, Deepak Srivastava wrote:
|
|
Seems done now. Let me know if you see anything else. Thanks. |
|
Test failure appears to be unrelated. I get a failure on the same test method locally without this PR applied. |
|
Yeah - that test failure started with the last merge from 4.6 |
|
Speaking of test failures ... doesn't look like there are any new tests in this PR... Pretty sure there is a starting point for testing merging via the api in the api_v3_ContactTest |
CRM-16865: Mass Dedupe Workflow Improvements
Includes work for https://issues.civicrm.org/jira/browse/CRM-16865.