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

feat: create a read-only contactdetails component #4194

Merged
merged 1 commit into from
Dec 18, 2024

Conversation

GretaD
Copy link
Contributor

@GretaD GretaD commented Oct 23, 2024

this will be the readonly component to be used on contacts and mail

this change is required for ref: nextcloud/mail#9622

@GretaD GretaD requested a review from st3iny October 23, 2024 12:48
@GretaD
Copy link
Contributor Author

GretaD commented Oct 23, 2024

@st3iny its still a draft, but if you already have comments, please let me know :)

@GretaD GretaD changed the title feat: create a readonly contactdetails feat: create a read-only contactdetails component Oct 23, 2024
@GretaD GretaD self-assigned this Oct 23, 2024
@GretaD GretaD added the 2. developing Work in progress label Oct 23, 2024
Copy link

codecov bot commented Oct 30, 2024

Codecov Report

Attention: Patch coverage is 0% with 115 lines in your changes missing coverage. Please review.

Project coverage is 11.37%. Comparing base (04a1d6f) to head (b5458b1).
Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
src/views/ReadOnlyContactDetails.vue 0.00% 88 Missing ⚠️
src/oca/mountContactDetails.js 0.00% 15 Missing ⚠️
lib/Listener/LoadContactsOcaApi.php 0.00% 7 Missing ⚠️
src/oca.js 0.00% 4 Missing ⚠️
lib/AppInfo/Application.php 0.00% 1 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##               main    #4194       +/-   ##
=============================================
- Coverage     66.20%   11.37%   -54.83%     
- Complexity      262      265        +3     
=============================================
  Files            25      121       +96     
  Lines           787     5556     +4769     
  Branches          0     1196     +1196     
=============================================
+ Hits            521      632      +111     
- Misses          266     4802     +4536     
- Partials          0      122      +122     

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

@GretaD GretaD added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Nov 26, 2024
@GretaD GretaD marked this pull request as ready for review November 26, 2024 14:34
@GretaD GretaD requested a review from kesselb November 26, 2024 14:34
@GVodyanov
Copy link
Contributor

Hey! How do I review this? Is it the section in the composer?

@GretaD
Copy link
Contributor Author

GretaD commented Dec 3, 2024

Hey! How do I review this? Is it the section in the composer?

you can check the code if you want, but the data you see on mail composer from the recipient, comes from here :)

@kesselb
Copy link
Contributor

kesselb commented Dec 3, 2024

  • Make filter work

I noticed that the report request always returns all cards instead of only the ones matching the filter. This seems to be caused by an issue with the XML request body, which is incorrect.

Wrong:

<x4:addressbook-query xmlns:x4="urn:ietf:params:xml:ns:carddav">
	<x0:prop xmlns:x0="DAV:">
... removed for better readability
	</x0:prop>
	<x4:filter test="anyof">
		<x4:prop-filter name="EMAIL">
			<x1:text-match xmlns:x1="urn:ietf:params:xml:ns:caldav">
				[email protected]
			</x1:text-match>
		</x4:prop-filter>
	</x4:filter>
</x4:addressbook-query>

Correct:

<x4:addressbook-query xmlns:x4="urn:ietf:params:xml:ns:carddav">
<x0:prop xmlns:x0="DAV:">
... removed for better readability
</x0:prop>
<x4:filter test="anyof">
	<x4:prop-filter name="EMAIL">
		<x4:text-match>[email protected]</x4:text-match>
	</x4:prop-filter>
</x4:filter>
</x4:addressbook-query>

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. See comments on the reference PR in description

@GretaD GretaD force-pushed the feat/RecipientDetails branch from c50ca7b to e25da15 Compare December 10, 2024 17:30
src/oca.js Outdated Show resolved Hide resolved
Signed-off-by: greta <[email protected]>
Signed-off-by: Richard Steinmetz <[email protected]>
@st3iny st3iny force-pushed the feat/RecipientDetails branch from e25da15 to b5458b1 Compare December 18, 2024 14:54
Copy link
Member

@st3iny st3iny left a comment

Choose a reason for hiding this comment

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

Tested and works.

The bundle is now loaded synchronously. The impact is not that bad.

image

@ChristophWurst
Copy link
Member

Thank you!

@ChristophWurst ChristophWurst added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Dec 18, 2024
@ChristophWurst ChristophWurst merged commit 8a1594f into main Dec 18, 2024
33 checks passed
@ChristophWurst ChristophWurst deleted the feat/RecipientDetails branch December 18, 2024 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants