nextcloud / contacts

📇 Contacts app for Nextcloud
https://apps.nextcloud.com/apps/contacts
GNU Affero General Public License v3.0
563 stars 169 forks source link

All contacts show *Last modified seconds ago* #3414

Open ChristophWurst opened 1 year ago

ChristophWurst commented 1 year ago

Describe the bug

Last modified date, if known, should work reliably

Steps to reproduce

  1. Open the contacts app
  2. Open any contact (can be your own, recently contacted or system address book)
  3. Scroll to the bottom

Expected behavior

Real last modified date

Actual behavior

image

Contact version

5.3

Operating system

No response

PHP engine version

None

Web server

None

Database

None

Additional info

No response

meichthys commented 1 year ago

I'm not seeing this in v 5.3.0-rc.2

Image

Could it be a caching issue?

meichthys commented 1 year ago

Maybe it's limited to recently contacted contacts?

ChristophWurst commented 1 year ago

I still see it with 27 RC2. It happens with my own contacts, recently contacted and system contacts

meichthys commented 1 year ago

Ok, for reference I'm on 26.0.1 RC1 I only have 1 recent contact but that does seem to have the issue, Other contacts are fine.

ChristophWurst commented 1 year ago

I tested again and this only happens with system contacts. A contact in a real address book shows Last edited 5 months ago

ChristophWurst commented 1 year ago

My impression is that we don't have a last modification date for every contact.

If we don't have the data we should hide the text

ChristophWurst commented 1 year ago

https://github.com/nextcloud/server/issues/38544 for a related feature request. The SAB contacts do not have a REV at the moment.

JohannesGGE commented 1 year ago

I found where the magic happens:

https://github.com/nextcloud/contacts/blob/6f79fbc0f324925c8f1f5a048e4f98eb2ccf3898/src/services/checks/invalidREV.js#L55-L69

The fix is called because the fallowing returns true in some cases:

https://github.com/nextcloud/contacts/blob/6f79fbc0f324925c8f1f5a048e4f98eb2ccf3898/src/services/checks/invalidREV.js#L31-L53

One of these cases are the vCard created by the contacts app:

BEGIN:VCARD
VERSION:4.0
PRODID:-//Nextcloud Contacts v5.3.0-beta2
UID:6aa9cf0f-e92e-4e5a-9c13-799ca265f9fd
FN:Contact
ADR;TYPE=HOME:;;;;;;
EMAIL;TYPE=HOME:
TEL;TYPE=HOME,VOICE:
REV;VALUE=DATE-AND-OR-TIME:20230531T135555Z
END:VCARD

REV;VALUE=DATE-AND-OR-TIME:20230531T135555Z seems to be a valid format for v4.0 of vCards but invalid for v3.0 as far as I understand it and as written in this issue https://github.com/nextcloud/contacts/issues/1352 . I don't understand why version === '3.0' && type === 'vcardtime' return true (valid but actually invalid) here, and 4.0 return false (invalid but it is actually valid)

Ical.js says revValue.icalclass === 'vcardtime' for REV;VALUE=DATE-AND-OR-TIME:20230531T135555Z and revValue.icalclass === 'icaltime' for REV:20230531T135555Z. Can't relate why.


So the actual problem is that the contacts app now writes vCard v4.0 and the fix wants to repair the valid REV. To fix the issue, I would just add the case version === '4.0' && type === 'vcardtime' where the validation returns valid. But as stated I don't really get the implementation here.

ChristophWurst commented 1 year ago

Does it also try to repair read-only system accounts?

I have to dig into details of vcard v3 and v4 too. @st3iny might know a few things too.

JohannesGGE commented 1 year ago

No, because SAB doesn't have REV yet, it will be set to now here: https://github.com/nextcloud/contacts/blob/bf01de83fd643fc16b80d92f220d49ed7915dec0/src/models/contact.js#L84-L89

st3iny commented 1 year ago

Is hiding the rev if it's missing a valid workaround for now?

ChristophWurst commented 1 year ago

https://github.com/nextcloud/contacts/blob/830f17933904384a1ec5e78e9a3b2d5fba463cd5/src/components/ContactDetails.vue#L271 I think that code tries that but if the repair kicks in and sets a new REV it will render again