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

Hide social sync option if social sync is not available #2913

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

Conversation

pboguslawski
Copy link

This mod hides Update avatars from social media (refreshed once per week)
checkbox from contacts settings if syncing avatars is disabled by admin
or no internet connection is avalable.

Author-Change-Id: IB#1125033
Related: #1594
Signed-off-by: Pawel Boguslawski pawel.boguslawski@ib.pl

This mod hides `Update avatars from social media (refreshed once per week)`
checkbox from contacts settings if syncing avatars is disabled by admin
or no internet connection is avalable.

Author-Change-Id: IB#1125033
Related: nextcloud#1594
Signed-off-by: Pawel Boguslawski <pawel.boguslawski@ib.pl>
Copy link
Member

@call-me-matt call-me-matt left a comment

Choose a reason for hiding this comment

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

code looks good, thank you for this improvement!
not sure if it is good practice to add your copyright to all the files where you have added a word.

@@ -4,6 +4,7 @@

/**
* @copyright 2017 Georg Ehrke <oc.list@georgehrke.com>
* @copyright Copyright (c) 2022 Informatyka Boguslawski sp. z o.o. sp.k., http://www.ib.pl/
Copy link
Member

Choose a reason for hiding this comment

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

triple copyright and no https? 😛

Copy link
Author

Choose a reason for hiding this comment

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

@ChristophWurst ChristophWurst added the 4. to release Ready to be released and/or waiting for tests to finish label Sep 14, 2022
@codecov
Copy link

codecov bot commented Sep 14, 2022

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 8.33%. Comparing base (030d2d5) to head (2b3d45e).
Report is 857 commits behind head on main.

❗ Current head 2b3d45e differs from pull request most recent head b741dd6. Consider uploading reports for the commit b741dd6 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##              main   #2913      +/-   ##
==========================================
+ Coverage     5.17%   8.33%   +3.15%     
==========================================
  Files          110      87      -23     
  Lines         1874    1164     -710     
  Branches       238     239       +1     
==========================================
  Hits            97      97              
+ Misses        1662     952     -710     
  Partials       115     115              

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

@ChristophWurst
Copy link
Member

CI isn't happy :)

@miaulalala
Copy link
Contributor

@pboguslawski ping :) can you fix the CI issues please?

you can lint with composer run cs:fix

and the SocialApiServiceTest fixing.

@ChristophWurst ChristophWurst added 1. to develop Accepted and waiting to be taken care of and removed 4. to release Ready to be released and/or waiting for tests to finish labels Oct 20, 2022
Fixes: 8a84821
Related: nextcloud#2913 (comment)
Author-Change-Id: IB#1125033
Signed-off-by: Pawel Boguslawski <pawel.boguslawski@ib.pl>
@pboguslawski
Copy link
Author

you can lint with composer run cs:fix

Fixed in 3aaf6ef.

and the SocialApiServiceTest fixing.

Not sure why this test fails. Maybe has_internet_connection is false during this test and should be set to true for this test? Not experienced in nc unit tests devel so please suggest fix.

@J0WI J0WI added 3. to review Waiting for reviews and removed 1. to develop Accepted and waiting to be taken care of labels Dec 14, 2022
@J0WI J0WI requested a review from ChristophWurst December 14, 2022 21:55
@ChristophWurst
Copy link
Member

I'm trying to update the branch to get CI running. Unfortunately the settingssection component was remvoed with #3032.

@ChristophWurst
Copy link
Member

I succeeded with the update but @pboguslawski's branch doesn't allow me to push.

@pboguslawski could you please update the branch yourself? You have to move the settingsection changes to src/components/AppNavigation/ContactsSettings.vue

@ChristophWurst ChristophWurst added 1. to develop Accepted and waiting to be taken care of and removed 3. to review Waiting for reviews labels Dec 16, 2022
@pboguslawski
Copy link
Author

I succeeded with the update but @pboguslawski's branch doesn't allow me to push.

@pboguslawski could you please update the branch yourself? You have to move the settingsection changes to src/components/AppNavigation/ContactsSettings.vue

Fixed in 2b3d45e.

@hamza221
Copy link
Contributor

hamza221 commented Apr 2, 2024

@pboguslawski friendly ping to update the copyright section

Don't overwrite, add yours instead:

Co-authored-by: Anna <anna@nextcloud.com>
Signed-off-by: Paweł Bogusławski <pawel.boguslawski@ib.pl>
@pboguslawski
Copy link
Author

Restored copyright note that was removed unintentionally (8f9d697).

This PR should not modify other Copyright notices.

Signed-off-by: Paweł Bogusławski <pawel.boguslawski@ib.pl>
@pboguslawski
Copy link
Author

After further investigation, removed this copyright in b741dd6 because it should not be modified in this PR (removal was effect of main merge and there is no such copyright in main now. As you can see if Files changed, this PR only adds our Copyright now.

@hamza221 hamza221 added 3. to review Waiting for reviews and removed 1. to develop Accepted and waiting to be taken care of labels Apr 2, 2024
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants