CRM-21433: Optimize dupe checking in Recent Items stack

[niels-heinemann]

Don't check for dupes by url beacause query params my change their order. Instead use type and id.

---

• CRM-21433: Optimize dupe checking in Recent Items stack

[mlutfy]

Hi,

Can you open a JIRA issue? https://docs.civicrm.org/dev/en/latest/core/contributing/

Also, can you provide examples of what would normally create a dupe, so that we can test before & after?

From a code style point of view, it seems to me like using === over == does not add any benefits in that case? (but makes the reader stop and wonder why === was used).

[eileenmcnaughton]

i think === is supposed to be what you should use unless there is a reason not to - e.g you think there is a risk of a number being passed as a string

[niels-heinemann]

Issue filed at https://issues.civicrm.org/jira/browse/CRM-21433.

I just produced some dupes while implementing #11280. In one case I injected an activity item with query params reset=1&id={$this->_activityId}&action=view and in another I changed the order to action=view&reset=1&id=$activityId without thinking about it.

== was a typo (by me ;) in a PR before and I fixed it here. It's just like @eileenmcnaughton said, === is the default way to go when trying to achieve kind of type safety.

[mlutfy]

Call me pedantic, but I am not able to reproduce the bug.

For example, I tried accessing:

• http://dmaster.demo.civicrm.org/civicrm/contact/view?reset=1&cid=202&t=1
• http://dmaster.demo.civicrm.org/civicrm/contact/view?reset=1&cid=202&t=2
• http://dmaster.demo.civicrm.org/civicrm/contact/view?cid=202&reset=1

And only one contact was shown in the Recent Items.

Am I missing something obvious? (the old code clearly seems wrong)

[niels-heinemann]

Yeah. It's not the request's query params order which is passed. It's the programmer's order at the place Recent.php's add-method is called! See

$url = CRM_Utils_System::url(implode("/", $this->urlPath), "reset=1&id={$activityId}&action=view& cid={$values['source_contact_id']}");
  CRM_Utils_Recent::add($this->_values['subject'],
	  $url,
              [...]
  );

[colemanw]

This makes sense to me.