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

[BE] Feature/ita profiles collaborators #106 and #107 #111

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from
Open

[BE] Feature/ita profiles collaborators #106 and #107 #111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
831324d
index() refactor ifelse to match expression. TODO collaboratorItaProf…
brycoti Dec 14, 2023
f3687a0
Itaprofiles() and wiki() updated
brycoti Dec 20, 2023
ba75e2b
refactor
brycoti Jan 8, 2024
c105a2f
.env variable used for token)
brycoti Jan 8, 2024
4f0aec8
PHP Linting (Pint)
brycoti Jan 8, 2024
611047a
swagger doc ready
brycoti Jan 17, 2024
bfa1dcd
Merge branch 'feature/ita-profiles-collaborators' of https://github.c…
brycoti Jan 17, 2024
98cf763
PHP Linting (Pint)
brycoti Jan 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions ...ions/OpenApi/controllersAnnotations/collaboratorsAnnotations/AnnotationsCollaborators.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,26 @@ class AnnotationsCollaborators
* path="/collaborators/{area}",
* tags={"Collaborators"},
* summary="User Collaborators",
* description="This endpoint is used to get persons that work on the specific project",
* description="This endpoint is used to get persons that work on the specific project,
* URL parameters: area (landing, wiki, challenges, profiles).",
Copy link
Collaborator

Choose a reason for hiding this comment

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

This line is not in the next line at Swagger, I mean, it's all togheter with an asterisc in the middle, at least in my browser.I've tried to fix it but I couldn't.

*
* @OA\Parameter(
* name="area",
* in="path",
* required=true,
* description="name of the area",
* description="name of the area.",
*
* @OA\Schema(
* type="string",
* enum={"landing", "wiki", "challenges", "profiles"},
* example="landing"
* )
* ),
* ),
*
* @OA\Response(
* response="404",
* description="Invalid area"
* ),
* @OA\Response(
* response="200",
* description="Collaborators details.",
Expand Down
24 changes: 20 additions & 4 deletions app/Http/Controllers/api/CollaboratorsController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ public function index($area)
return $this->collaboratorItaWiki();
} elseif ($area === 'challenges') {
return $this->collaboratorItaChallenges();
} elseif ($area === 'profiles') {
return $this->collaboratorItaProfiles();
}

return response()->json([
Copy link
Collaborator

Choose a reason for hiding this comment

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

I've found another way to approach this:

Suggested change
return response()->json([
$projects = ['landing', 'wiki', 'challenges', 'profiles'];
if (!in_array($area, $projects)) {
return response()->json([
'message' => 'this area is invalid',
], 404);
} else {
$methodName = 'collaboratorIta' . ucfirst($area);
return $this->{$methodName}();
}```
I can't add the suggestion at L10. IDK way.

Expand All @@ -24,7 +26,6 @@ public function index($area)

public function collaboratorLogic($collaborator)
{

$url = env('URL_SERVER_API', 'https://api.github.com');
$response = Http::withToken(env('MY_TOKEN'))->get($url.$collaborator);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't the token ba passed by a query parameter? I reckon frontend users won't be able to edit .env file.


Expand All @@ -40,7 +41,6 @@ public function collaboratorLogic($collaborator)
}

return $allCollaborators;

}

public function uniqueCollaborators(...$arrays)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this function it's not needed by aplicate the change suggested at L61. And that change can be applied at the rest of methods

Expand All @@ -60,7 +60,6 @@ public function uniqueCollaborators(...$arrays)

public function collaboratorLanding()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
public function collaboratorLanding()
public function collaboratorLanding()
{
$collaboratorPhp = '/ita-landing-backend/collaborators?affiliation=direct';
$collaboratorReact = '/ita-landing-frontend/collaborators?affiliation=direct';
$php = $this->collaboratorLogic($collaboratorPhp);
$react = $this->collaboratorLogic($collaboratorReact);
// Return unique (not repeated) collaborators from php and react projects.
return array_unique(array_merge($php, $react));
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to change the method name if suggestion at index() is accepted.

Suggested change
public function collaboratorLanding()
public function collaboratorItaLanding()

{

$collaboratorPhp = '/ita-landing-backend/collaborators?affiliation=direct';
$collaboratorReact = '/ita-landing-frontend/collaborators?affiliation=direct';

Expand All @@ -78,7 +77,11 @@ public function collaboratorItaWiki()

$collaboratorWiki = '/ita-wiki/collaborators?affiliation=direct';

return $this->collaboratorLogic($collaboratorWiki);
$wiki = $this->collaboratorLogic($collaboratorWiki);

$uniqueCollaborators = $this->uniqueCollaborators($wiki);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Applying the L61 suggestion we also avoid the following SonarCloud message:
"Immediately return this expression instead of assigning it to the temporary variable "$uniqueCollaborators"."


return $uniqueCollaborators;

}

Expand All @@ -95,4 +98,17 @@ public function collaboratorItaChallenges()

return $uniqueCollaborators;
}

public function collaboratorItaProfiles()
{
$collaboratorReactProfiles = '/ita-profiles-frontend/collaborators?affiliation=direct';
$collaboratorPhpProfiles = '/ita-profiles-backend/collaborators?affiliation=direct';

$reactProfiles = $this->collaboratorLogic($collaboratorReactProfiles);
$phpProfiles = $this->collaboratorLogic($collaboratorPhpProfiles);

$uniqueCollaborators = $this->uniqueCollaborators($reactProfiles, $phpProfiles);

return $uniqueCollaborators;
}
}
19 changes: 17 additions & 2 deletions storage/api-docs/api-docs.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,21 +348,30 @@
"Collaborators"
],
"summary": "User Collaborators",
"description": "This endpoint is used to get persons that work on the specific project",
"description": "This endpoint is used to get persons that work on the specific project,\n * URL parameters: area (landing, wiki, challenges, profiles).",
"operationId": "9ed89cb2a49abd38189ba3c947d6c40f",
"parameters": [
{
"name": "area",
"in": "path",
"description": "name of the area",
"description": "name of the area.",
"required": true,
"schema": {
"type": "string",
"enum": [
"landing",
"wiki",
"challenges",
"profiles"
],
"example": "landing"
}
}
],
"responses": {
"404": {
"description": "Invalid area"
},
"200": {
"description": "Collaborators details.",
"content": {
Expand Down Expand Up @@ -814,6 +823,12 @@
],
"example": "ADMIN"
},
"last_login_at": {
"description": "Last user log in timestamp",
"type": "string",
"format": "date-time",
"example": "2023-06-19T12:00:00+00:00"
},
"email_verified_at": {
"description": "Email verification timestamp",
"type": "string",
Expand Down
14 changes: 14 additions & 0 deletions tests/Feature/CollaboratorTest.php
View file
Open in desktop
Copy link
Collaborator

@FranEnLaNube FranEnLaNube Jan 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

If CollaboratorController::collaboratorLanding() is changed as suggested, we may don't forget to change it here at L57

Copy link
Collaborator

Choose a reason for hiding this comment

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

I couldn't check these tests due the well-known problem with the token of the IT-Academy repository.
Perhaps could be a good idea to approach this by using mocking.

Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ public function test_index_get_collaborators_challenges(): void
$response->assertStatus(200);
}

public function test_index_get_collaborators_profiles(): void
{
$response = $this->get('/api/collaborators/profiles');
$response->assertStatus(200);
}

public function test_inserting_on_index_a_area_that_dont_exist(): void
{
$response = $this->get('/api/collaborators/prueba');
Expand Down Expand Up @@ -67,4 +73,12 @@ public function test_collaborator_challenges_function()
$response = $collaboratorsController->collaboratorItaChallenges();
$this->assertIsArray($response);
}

public function test_collaborator_ita_profiles_function()
{

$collaboratorsController = new CollaboratorsController();
$response = $collaboratorsController->collaboratorItaProfiles();
$this->assertIsArray($response);
}
}