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

Move users_picker profile custom picker to contacts #4231

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

julien-nc
Copy link
Member

@julien-nc julien-nc commented Nov 29, 2024

This replaces #3487. I can't push in the branch there.

@ChristophWurst

  • It's rebased on main
  • Adjusted the vite config
  • The bio processing is now using multi-byte proof functions
  • Nitpick addressed: The condition in the reference provider was inverted
  • Now checks if the profile is enabled (when resolving a link)
  • Fixed widget style issues
  • Fixed the tests
  • Fixed the licenses

About #3487 (review) I'm not sure I see the issue. The frontend is using the ocs/v2.php/core/autocomplete/get endpoint which should respect the sharing settings.

The reference provider now checks if the profile is enabled or not with the IAccountManager::PROPERTY_PROFILE_ENABLED account property.
It seems fine to resolve any /u/USER_ID link as long as the profile is enabled.

@julien-nc julien-nc added enhancement New feature or request 3. to review Waiting for reviews labels Nov 29, 2024
@SebastianKrupinski
Copy link
Contributor

Hi @julien-nc

How do I test? What am I looking for.

@julien-nc
Copy link
Member Author

@SebastianKrupinski This adds a "profile picker" entry in the smart picker and a reference widget (link preview) for /u/USER_ID links.

@julien-nc julien-nc force-pushed the enh/noid/profile-picker-move branch from a705e1a to cfa86a0 Compare December 4, 2024 13:41
@julien-nc julien-nc force-pushed the enh/noid/profile-picker-move branch from cfa86a0 to 23fc538 Compare December 4, 2024 13:43
Signed-off-by: Julien Veyssier <[email protected]>
@julien-nc julien-nc force-pushed the enh/noid/profile-picker-move branch from bde8497 to 69b73de Compare December 4, 2024 13:52
Signed-off-by: Julien Veyssier <[email protected]>
@julien-nc julien-nc force-pushed the enh/noid/profile-picker-move branch from 9090d67 to 4fb561f Compare December 4, 2024 14:15
Copy link

codecov bot commented Dec 4, 2024

Codecov Report

Attention: Patch coverage is 39.03743% with 114 lines in your changes missing coverage. Please review.

Project coverage is 12.48%. Comparing base (430505e) to head (a7cc035).
Report is 27 commits behind head on main.

Files with missing lines Patch % Lines
.../components/ProfilePicker/ProfilesCustomPicker.vue 0.00% 60 Missing ⚠️
...nts/ProfilePicker/ProfilePickerReferenceWidget.vue 0.00% 24 Missing ⚠️
src/reference.js 0.00% 17 Missing ⚠️
lib/Reference/ProfilePickerReferenceProvider.php 91.25% 7 Missing ⚠️
lib/Listener/ProfilePickerReferenceListener.php 0.00% 4 Missing ⚠️
lib/AppInfo/Application.php 0.00% 2 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##               main    #4231       +/-   ##
=============================================
- Coverage     66.33%   12.48%   -53.85%     
- Complexity      263      290       +27     
=============================================
  Files            25      122       +97     
  Lines           799     5647     +4848     
  Branches          0     1216     +1216     
=============================================
+ Hits            530      705      +175     
- Misses          269     4821     +4552     
- Partials          0      121      +121     

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

@julien-nc
Copy link
Member Author

julien-nc commented Dec 4, 2024

@SebastianKrupinski I made a few more adjustments. CI is green. It is now ready to be tested.

@ChristophWurst
Copy link
Member

Dear @julien-nc,

Thank you for your contribution!

About #3487 (review) I'm not sure I see the issue. The frontend is using the ocs/v2.php/core/autocomplete/get endpoint which should respect the sharing settings.

That is correct for the part of searching. But once a reference is selected, there is no more check in ProfilePickerReferenceProvider.
I could abuse that to render a reference to a user I would otherwise not see. The reference contains any non-private data.

I suggest we test this specific scenario and see how the code behaves. I just want to have clarity on this point before the code goes into the main branch.

@ChristophWurst
Copy link
Member

@julien-nc on a higher level, what do you think about moving the app/code into server instead? The picker picks users, not contacts. There is nothing specific to contacts (or event teams) in the code.

@julien-nc julien-nc force-pushed the enh/noid/profile-picker-move branch 2 times, most recently from a8bdf1e to 686282e Compare December 5, 2024 16:39
@julien-nc
Copy link
Member Author

@ChristophWurst Yes we can move it to server.

I'd still like to have your opinion on the last changes. We now check the visibility of the profile fields with the ProfileManager. This respects the scopes and the visibility settings.

I could abuse that to render a reference to a user I would otherwise not see. The reference contains any non-private data.

Now the preview can't display any information that would not be visible by clicking the link to browse the profile page.

@julien-nc julien-nc force-pushed the enh/noid/profile-picker-move branch from 686282e to cbc1051 Compare December 17, 2024 11:51
@julien-nc julien-nc force-pushed the enh/noid/profile-picker-move branch from cbc1051 to a7cc035 Compare December 17, 2024 12:00
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 enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants