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.

Sign up for GitHub

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

Jump to bottom

change Mastodon tree for social avatar #3504

Merged
merged 1 commit into from
Jul 27, 2023
Merged

change Mastodon tree for social avatar #3504

merged 1 commit into from
Jul 27, 2023

Conversation

call-me-matt
Copy link
Member

Mastodon avatars seem to be at a different location in the json tree. This fix adapts to the change.

@call-me-matt call-me-matt force-pushed the fix/social-mastodon branch 3 times, most recently from 6598f26 to 434de78 Compare July 6, 2023 22:51
@call-me-matt call-me-matt self-assigned this Jul 6, 2023
@codecov
Copy link

codecov bot commented Jul 6, 2023 •
edited
Loading

Codecov Report

Patch coverage has no change and project coverage change: -1.82% ⚠️

Comparison is base (cecab50) 1.81% compared to head (de0d27a) 0.00%.
Report is 43 commits behind head on main.

❗ Current head de0d27a differs from pull request most recent head 115b1b4. Consider uploading reports for the commit 115b1b4 to get more accurate results

Additional details and impacted files 
@@            Coverage Diff             @@
##              main   #3504      +/-   ##
==========================================
- Coverage     1.81%   0.00%   -1.82%     
- Complexity     272     275       +3     
==========================================
  Files          113      25      -88     
  Lines         6123     830    -5293     
  Branches      1480       0    -1480     
==========================================
- Hits           111       0     -111     
+ Misses        5896     830    -5066     
+ Partials       116       0     -116
Files Changed Coverage Δ
lib/Service/Social/MastodonProvider.php 0.00% <0.00%> (ø)

... and 88 files with indirect coverage changes

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

@call-me-matt call-me-matt added 3. to review Waiting for reviews bug Something isn't working labels Jul 6, 2023
ChristophWurst
ChristophWurst reviewed Jul 7, 2023
View reviewed changes
lib/Service/Social/MastodonProvider.php Outdated
Comment on lines 82 to 84
$result = $this->httpClient->get($profileUrl, ['headers' => ['Accept' => 'application/json']]);
$jsonResult = json_decode($result->getBody());
return $jsonResult->avatar;
return $jsonResult->icon->url;
Copy link
Member

Choose a reason for hiding this comment

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

nit: deserialize into an associative array and use the null coalesce operator to not cause errors with missing keys

Copy link
Member Author

Choose a reason for hiding this comment

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

good comment. I modfied it as follows:

contacts/lib/Service/Social/MastodonProvider.php

Line 83 in 068c135

$jsonResult = json_decode($result->getBody(), true);

@call-me-matt call-me-matt marked this pull request as draft July 7, 2023 23:50
@call-me-matt call-me-matt added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Jul 8, 2023
@call-me-matt call-me-matt force-pushed the fix/social-mastodon branch 2 times, most recently from 896d1d9 to 980d581 Compare July 10, 2023 21:26
@call-me-matt call-me-matt added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jul 10, 2023
@call-me-matt call-me-matt marked this pull request as ready for review July 10, 2023 21:28
tcitworld
tcitworld approved these changes Jul 13, 2023
View reviewed changes
lib/Service/Social/MastodonProvider.php Outdated Show resolved Hide resolved
@call-me-matt call-me-matt force-pushed the fix/social-mastodon branch 2 times, most recently from 7d3a2cb to 6712b78 Compare July 13, 2023 10:48
@call-me-matt
Copy link
Member Author

@ChristophWurst what is missing in order to merge?

st3iny
st3iny approved these changes Jul 27, 2023
View reviewed changes
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. Code looks good to me.

Thanks for the PR :)

@st3iny
Copy link
Member

st3iny commented Jul 27, 2023

PS: It seems that the committer and signed-off names are diverging. @call-me-matt Could you please fix it so that DCO passes?

@call-me-matt call-me-matt merged commit 18819a5 into main Jul 27, 2023
26 checks passed
@call-me-matt call-me-matt deleted the fix/social-mastodon branch July 27, 2023 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tcitworld tcitworld tcitworld approved these changes

@st3iny st3iny st3iny approved these changes

@ChristophWurst ChristophWurst Awaiting requested review from ChristophWurst

Assignees

@call-me-matt call-me-matt

Labels
3. to review Waiting for reviews bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@call-me-matt @st3iny @ChristophWurst @tcitworld