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

Add ability to set relationships through the api #1181

Merged
merged 16 commits into from
Apr 17, 2018
Merged

Add ability to set relationships through the api #1181

merged 16 commits into from
Apr 17, 2018

Conversation

djaiss
Copy link
Member

@djaiss djaiss commented Apr 15, 2018 •
edited
Loading

Code-related tasks

  • Tests added for this feature/bug.
  • Impact on the seeders.
  • Impact on the API.

If the code changes the SQL schema

  • Impact on account export.
  • Impact on importing data with vCard and .csv files.
  • Impact on account reset and deletion.

Other tasks

  • CHANGELOG entry added, if necessary, under UNRELEASED.
  • CONTRIBUTORS entry added, if necessary.
  • If it's relevant and worth mentioning, create a changelog entry for this change. The changelog entry will appear inside the UI for all users to see. To know if your change is worth the creation of a changelog entry, read the documentation.
  • Indicate [wip] in the title of the PR it is is not final yet. Remove [wip] when ready. Otherwise the PR will be considered complete and rejected if it's not working.

@djaiss djaiss added this to the 2.0.1 milestone Apr 15, 2018
@asbiin
Copy link
Member

asbiin commented Apr 15, 2018

[test]
capture
tests/Browser/screenshots/failure-Tests_Browser_Settings_MultiFAControllerTest_testEnable2faWrongCode-0.png
[id:Blwp9inebM5Y1BS]

@codecov
Copy link

codecov bot commented Apr 15, 2018 •
edited
Loading

Codecov Report

Merging #1181 into master will increase coverage by 1.5%.
The diff coverage is 82.8%.

Impacted file tree graph

@@            Coverage Diff             @@
##             master   #1181     +/-   ##
==========================================
+ Coverage        41%   42.5%   +1.5%     
  Complexity       11      11             
==========================================
  Files           203     204      +1     
  Lines          5275    5360     +85     
==========================================
+ Hits           2162    2276    +114     
+ Misses         3113    3084     -29
Impacted Files Coverage Δ Complexity Δ
app/Relationship.php 100% <ø> (ø) 0 <0> (ø) ⬇️
.../Resources/Relationship/RelationshipCollection.php 0% <0%> (ø) 0 <0> (?)
app/Http/Controllers/Api/ApiController.php 82.6% <100%> (+9.4%) 0 <0> (ø) ⬇️
...Http/Controllers/Api/ApiRelationshipController.php 83.4% <84.4%> (+83.4%) 0 <0> (ø) ⬇️
app/Reminder.php 100% <0%> (+2.1%) 0% <0%> (ø) ⬇️
app/Http/Resources/Relationship/Relationship.php 100% <0%> (+100%) 0% <0%> (ø) ⬇️
app/Http/Resources/Contact/ContactShort.php 100% <0%> (+100%) 0% <0%> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4f88e5a...b3c5e9d. Read the comment docs.

@djaiss djaiss changed the title Add ability to set relationships through the api [wip] Add ability to set relationships through the api Apr 17, 2018
asbiin
asbiin reviewed Apr 17, 2018
View reviewed changes
Copy link
Member

@asbiin asbiin left a comment

Choose a reason for hiding this comment

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

SonarQube analysis found issues:
Bug Bugs: 0
Vulnerability Vulnerabilities: 0
Code Smell Code Smells: 2

See all issues in SonarCloud

app/Http/Controllers/Api/ApiRelationshipController.php
* @param Request $request
* @return mixed
*/
private function validateUpdateParameters(Request $request, $relationshipId)
Copy link
Member

Choose a reason for hiding this comment

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

Code Smell Code Smell: Reduce the number of returns of this function 4, down to the maximum allowed 3. (php:S1142)

See it in SonarCloud

app/Http/Controllers/Api/ApiRelationshipController.php
* @param Request $request
* @return mixed
*/
private function validateParameters(Request $request)
Copy link
Member

Choose a reason for hiding this comment

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

Code Smell Code Smell: Reduce the number of returns of this function 5, down to the maximum allowed 3. (php:S1142)

See it in SonarCloud

@djaiss djaiss merged commit 08f19c8 into master Apr 17, 2018
@djaiss djaiss deleted the 2018-04-15-relationship-api branch April 21, 2018 16:48
@github-actions
Copy link

This pull request has been automatically locked since there
has not been any recent activity after it was closed.
Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@asbiin asbiin asbiin left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@djaiss @asbiin