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 created_date & modified_date to civicrm_relationship #22480

Merged
merged 1 commit into from Jan 19, 2022

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Add created_date & modified_date to civicrm_relationship

As with other tables we have done this to, knowing when we added a record to the
DB is useful (and differs from the relationship start_date).

I'm not sure yet if I will try to expose in the UI or whether search-kit
is all we need

Before

void

After

Tracking

Technical Details

This is consistent with similar changes made for other tables

Comments

@civibot
Copy link

civibot bot commented Jan 12, 2022

(Standard links)

@civibot civibot bot added the master label Jan 12, 2022
@eileenmcnaughton eileenmcnaughton force-pushed the create_date branch 2 times, most recently from 2c309f3 to d0ebce5 Compare January 18, 2022 22:13
<comment>Relationship created date.</comment>
<required>true</required>
<default>CURRENT_TIMESTAMP</default>
<add>5.36</add>
Copy link
Contributor

Choose a reason for hiding this comment

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

5.47

Copy link
Contributor Author

Choose a reason for hiding this comment

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

opps

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixec

@colemanw
Copy link
Member

colemanw commented Jan 19, 2022

This will work, but by itself it will not be available to SearchKit, because the civicrm_relationship table is not available to SearchKit (it queries the civicrm_relationship_cache table).

On a related note, @MegaphoneJon requested a few other columns be searchable. https://lab.civicrm.org/dev/core/-/issues/3019

With all of these, there are 2 possible solutions:

  1. Add the mirror column to civicrm_relationship_cache
  2. Add a calculated field to the APIv4 RelationshipCache entity which performs a subquery join to fetch the value.

The 1st is probably faster to query (though maybe not by a wide margin), but the 2nd would keep the size of the civicrm_relationship_cache table down. I think I'll take a stab at the 2nd, as it can always be switched over to the 1st later without SearchKit noticing the difference, as the field names would be the same.

*/
public static function updateRelationshipDates(CRM_Queue_TaskContext $ctx): bool {
CRM_Core_DAO::executeQuery('
UPDATE civicrm_relationship SET created_date = start_date, modified_date = start_date WHERE start_date IS NOT NULL
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't have a specific example where start_date is less than 1970-01-01 GMT but I've seen it elsewhere and know how to make it. This should then crash because timestamp can't be less than that, whereas start_date is a date which can be.

It's escaping me at the moment but something like UPDATE ... SET created_date = MAX('1970-01-01', start_date)... would work except there is no such SQL function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good spotting - since the update is just updating to have a more useful starting value than 'right now' I added to the WHERE clause WHERE start_date IS NOT NULL AND start_date > "1970-01-01"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's arguabe that I should update it at all - but I think it's fine with the update

Copy link
Member

Choose a reason for hiding this comment

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

Yea I guess start_date often does correspond to when the relationship was created... but sometimes it doesn't.
But for sites with advanced logging enabled, both created and updated dates should be available in the log tables somewhere. Wouldn't that be more accurate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@colemanw yes they are there - but it is a bit of a PITA to grab them - do you think it is that important to get the starting value right - it will be right going forwards & that will date quickly enough

Copy link
Member

Choose a reason for hiding this comment

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

It's not that important imo, no

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK _ I think this is mergeable - it adds the columns - but it doesn't expose them anywhere....

As with other tables we have done this to, knowing when we added a record to the
DB is useful (and differs from the relationship start_date).

I'm not sure yet if I will try to expose in the UI or whether search-kit
is all we need
@colemanw colemanw merged commit c1b2ece into civicrm:master Jan 19, 2022
@colemanw colemanw deleted the create_date branch January 19, 2022 22:50
@colemanw
Copy link
Member

@eileenmcnaughton this exposes the fields to SearchKit: #22606

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