Default the date last modified to the creation date in the charts collection

AMP-SCZ / dpdash

Deep phenotyping dashboard

Other

2 stars 1 forks source link

Default the date last modified to the creation date in the charts collection #704

Closed Elkrival closed 8 months ago

Elkrival commented 8 months ago

closes #666

This pr makes the createdAt property the default value to display on the Modified column of the charts table.

mikestone14 commented 8 months ago

Is that because dayjs is defaulting to today? So it renders but could be inaccurate?

On Fri, Mar 22, 2024 at 11:41 AM Ivan Trujillo Ayala < @.***> wrote:

@.**** commented on this pull request.

In views/tables/ChartsTable/index.jsx https://github.com/AMP-SCZ/dpdash/pull/704#discussion_r1535792794:

return chart[property] ? dayjs(chart[property]).format(DATE_FORMAT) : ''

return dayjs(chart[property]).format(DATE_FORMAT)

Screenshot.2024-03-22.at.9.41.14.AM.png (view on web) https://github.com/AMP-SCZ/dpdash/assets/19805355/ef6d67b1-145a-4a9d-acdd-69759e01a874 looks fine to me

— Reply to this email directly, view it on GitHub https://github.com/AMP-SCZ/dpdash/pull/704#discussion_r1535792794, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAWFIOLA4DUYJIGIYYFVKL3YZRGLPAVCNFSM6AAAAABFAFHVRGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTSNJVGE2DMMBQGE . You are receiving this because you commented.Message ID: @.***>

Elkrival commented 8 months ago

we don't do that in the code to display this date

mikestone14 commented 8 months ago

we don't do that in the code to display this date

What does this mean? If it's related to this line

dayjs(chart[property]).format(DATE_FORMAT)

then if chart[property] is undefined we are defaulting to today (I believe) dayjs().format(DATE_FORMAT)

Elkrival commented 8 months ago

we don't do that in the code to display this date

What does this mean? If it's related to this line
dayjs(chart[property]).format(DATE_FORMAT)
then if chart[property] is undefined we are defaulting to today (I believe) dayjs().format(DATE_FORMAT)

is this the current code or what's in pr..? the current code has this return chart[property] ? dayjs(chart[property]).format(DATE_FORMAT) : '' and we will always have this property going forward... i'm confused about what you're asking...

mikestone14 commented 8 months ago

we don't do that in the code to display this date

What does this mean? If it's related to this line
dayjs(chart[property]).format(DATE_FORMAT)
then if chart[property] is undefined we are defaulting to today (I believe) dayjs().format(DATE_FORMAT)
is this the current code or what's in pr..? the current code has this return chart[property] ? dayjs(chart[property]).format(DATE_FORMAT) : '' and we will always have this property going forward... i'm confused about what you're asking...

this is what is in PR, not the current code. I think we want to keep the current code so we don't default the updated at day to today when it doesn't exist in the DB.

Elkrival commented 8 months ago

I understand your concern, we will be safe from this.

When we migrate charts we can add the udpatedAt property to the day of migration
With this pr we will always have this property going forward

honestly we probably won't ever need it, especially going forward

Elkrival commented 8 months ago

4461737 @mikestone14