Do not show Activity Separation on when viewing an Activity

[19ATF72]

Overview

Activity separation fields show up when viewing an activity.

Before

After

Steps to replicate

1.Find and open a profile of the contact from the precondition
2.Click "Actions" → "Meeting"
3.Click "Save"
4.Open the created meeting (Click "View")
5.Take a look at the "Activity Separation" field

[civicrm-builder]

Can one of the admins verify this patch?

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[seamuslee001]

add to whitelist

[eileenmcnaughton]

I agree with this - @demeritcowboy ?

[demeritcowboy]

@eileenmcnaughton I saw this one but didn't personally see it as a "problem". It is visually awkward and serves no purpose though if it's not going to show you what the original choice was. I can take a look.

[eileenmcnaughton]

thanks @demeritcowboy it does look odd & I guess that option is mostly about creating the activity rather than a 'feature' of it

[demeritcowboy]

Just see the note below for tech.

• General standards
  • [r-explain] PASS
  • [r-user] PASS
  • [r-doc] PASS
  • [r-run] PASS
• Developer standards
  • [r-tech] Issue
    • There's currently a conditional in the .php file as to whether this field is shown or not (for activity edit). Rather than split the conditionals between two files, would it be better to update this line: https://github.com/civicrm/civicrm-core/blob/5.16.2/CRM/Activity/Form/Activity.php#L747
  • [r-code] PASS
  • [r-maint] Undecided
  • [r-test] PASS

[demeritcowboy]

And that is now the 6th time I've seen that (unrelated) test failure - see https://lab.civicrm.org/dev/core/issues/938.

[19ATF72]

@demeritcowboy Updated changes to reflect your suggestion.
Can you please check it?
Thanks

[demeritcowboy]

Looks good. Thanks @19ATF77.

A separate note: In looking at that line it was bugging me "why isn't it just ==ADD"? It seems that originally it was but was changed consciously but without too much explanation: See ef89f22

Based on the jira issue maybe it has something to do with when you do a contact search and then select multiple and choose Add Activity? That is still working but then has an unrelated notice which is also present before the patch:
Notice: Undefined index: allowRepeatConfigToSubmit in CRM_Core_Form_RecurringEntity::postProcess() (line 346 of /.../web/sites/all/modules/civicrm/CRM/Core/Form/RecurringEntity.php)

Anyway, something for another day.

[yashodha]

@19ATF77
I second @demeritcowboy comment - looks like option should be there for ADD action.

[demeritcowboy]

@yashoda Just checking since the word add is confusing here - you mean that it's currently a negative comparison so that the search results dropdown choice for add activity works, not that you're saying the line should be changed back to only compare against ADD. Right?

[eileenmcnaughton]

@yashodha @demeritcowboy looks like this is agreed & mergeable?

[demeritcowboy]

From my point of view yes. It r-runs properly as is in all situations I tested. Any other questions could be dealt with separately.

[eileenmcnaughton]

ok thanks @yashodha @demeritcowboy @19ATF77

[eileenmcnaughton]

Is there a gitlab to close?

[19ATF72]

No gitlab, and thanks for the time.

[demeritcowboy]

To update on the earlier discussion about whether the line should be just ==ADD, the action is actually CRM_Core_Action::BASIC in the situation where you are creating from the contact search results page, so that explains that. I've added ticket https://lab.civicrm.org/dev/core/issues/1192 for the E_NOTICE.