Skip to content

Conversation

@st3iny
Copy link
Member

@st3iny st3iny commented Aug 13, 2025

Fix #4603

I added some compatibility code to keep the old route working (in case another app generates a link to the direct route).

How to test:

  1. Import the contacts below.
  2. For each contact:
  3. Navigate to it in the frontend.
  4. Reload the page.

Expected: The contacts app should open and show the contact.
Actual: A generic 404 page is shown.

Example contacts to test with:

BEGIN:VCARD
VERSION:3.0
PRODID:-//Sabre//Sabre VObject 4.5.6//EN
UID:KDplLQM7P/2JvRLqdpbziA==
REV;VALUE=DATE-TIME:20250806T093806Z
FN:UID Test
END:VCARD
BEGIN:VCARD
VERSION:3.0
PRODID:-//Sabre//Sabre VObject 4.5.6//EN
UID:26762ad9-c2ab-4f68-bd4c-6ff97267cf75-é
REV;VALUE=DATE-TIME:20250813T150607Z
FN:unicode test
END:VCARD

(Both will break on main and work here.)

@st3iny st3iny self-assigned this Aug 13, 2025
@st3iny st3iny added the 3. to review Waiting for reviews label Aug 13, 2025
@codecov
Copy link

codecov bot commented Aug 13, 2025

Codecov Report

❌ Patch coverage is 18.75000% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 9.69%. Comparing base (7d0596e) to head (37f4b12).
⚠️ Report is 255 commits behind head on main.

Files with missing lines Patch % Lines
src/components/ContactsList.vue 0.00% 3 Missing ⚠️
src/views/Contacts.vue 0.00% 3 Missing ⚠️
src/components/ContactsList/ContactsListItem.vue 0.00% 2 Missing ⚠️
src/mixins/RouterMixin.js 0.00% 2 Missing ⚠️
...c/components/AppNavigation/GroupNavigationItem.vue 0.00% 1 Missing ⚠️
...mponents/ContactDetails/ContactDetailsProperty.vue 0.00% 1 Missing ⚠️
src/models/contact.js 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              main   #4615      +/-   ##
==========================================
+ Coverage     9.65%   9.69%   +0.04%     
- Complexity     265     266       +1     
==========================================
  Files          126     126              
  Lines         6320    6314       -6     
  Branches      1180    1180              
==========================================
+ Hits           610     612       +2     
+ Misses        5588    5580       -8     
  Partials       122     122              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ChristophWurst
Copy link
Member

While IMO other apps should not use another app's route because this is a form of inter-app dependency, there is https://github.com/search?q=org%3Anextcloud%20contacts.contacts.direct&type=code

@st3iny
Copy link
Member Author

st3iny commented Aug 21, 2025

While IMO other apps should not use another app's route because this is a form of inter-app dependency, there is github.com/search?q=org%3Anextcloud%20contacts.contacts.direct&type=code

Right, that is a good point. I'll convert this to a fix then.

I added some compatibility code to keep the old routes working:

// Keep compatibility with old routing scheme
if (str_contains($contact, '~')) {
$contact = base64_encode($contact);
}

@st3iny st3iny force-pushed the feat/base64-routing branch from c75083b to 9c87b4e Compare August 21, 2025 08:06
@st3iny st3iny changed the title feat: routing based on base64 encoding fix: routing based on base64 encoding Aug 21, 2025
@st3iny
Copy link
Member Author

st3iny commented Aug 22, 2025

/backport to stable7.2

Signed-off-by: Richard Steinmetz <richard@steinmetz.cloud>
@st3iny st3iny force-pushed the feat/base64-routing branch from b86e6db to 37f4b12 Compare August 22, 2025 14:31
@st3iny
Copy link
Member Author

st3iny commented Aug 22, 2025

Rebased to fix conflicts.

@st3iny st3iny added the bug Something isn't working label Aug 22, 2025
@hamza221
Copy link
Contributor

/backport to stable7.3

@hamza221
Copy link
Contributor

/backport to stable8.0

Copy link
Contributor

@SebastianKrupinski SebastianKrupinski left a comment

Choose a reason for hiding this comment

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

Tested. Works good.

@github-actions
Copy link

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews bug Something isn't working feedback-requested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Slashes in vCard UIDs break routing

5 participants