SHARE-584 Introduction Task - Simon #5053
Conversation
This is my Introduction Task and I changed the ProjectController.php and project_details.html.twig to add a steal button. I changed the codeView Button to the steal button.
| 'text': "codeview.title"|trans({}, "catroweb") | ||
| 'text': "Steal" |
There was a problem hiding this comment.
Since you have not used the translation system, just a heads up for the future:
codeview.title is registered by the developer in the translations/catroweb.en.yaml file as
codeview:
title: Steal
All other translation files; E.g catroweb.de.yml are translated in its own process (automated by GitHub Actions) using Crowdin -> More details here: https://catrobat.atlassian.net/wiki/spaces/CATWEB/pages/74383377/Crowdin+-+Translation+Management
| @@ -212,6 +211,31 @@ public function projectLike(Request $request, string $id): Response | |||
| ]); | |||
| } | |||
|
|
|||
| #[Route(path: '/projectStealButton/{id}', name: 'projectStealButton', methods: ['GET'])] | |||
There was a problem hiding this comment.
At Catroweb we try to write our code using the API approach, to decouple the frontend from the backend. We have multiple different frontends (ours on the share, and also the apps)
Usually, your request (steal) would use our API (https://github.com/Catrobat/Catroweb-API // https://developer.catrobat.org/Catroweb-API/) Of course that is an oerhead for the training example.
Instead of using a GET request, consider using POST which is used to send data to a server to create/update a resource. Feel free to take a look at all available methods: https://www.w3schools.com/tags/ref_httpmethods.asp
That said it would be nice, to return a status code rather than a redirect in the response, and handle the response using the frontend logic.
| $project->setUser($user); | ||
| $this->entity_manager->flush(); | ||
|
|
||
| $this->addFlash('snackbar', 'Project stolen successfully!'); |
There was a problem hiding this comment.
what about the case, when the user already owns the project?
With |
| @@ -63,11 +63,11 @@ | |||
| </div> | |||
| <div class="col-6 mb-3 d-lg-none mt-3"> | |||
| {% include 'components/redirect_button.twig' with { | |||
There was a problem hiding this comment.
Usually, a new feature should not replace an existing feature ;)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #5053 +/- ##
=============================================
+ Coverage 40.14% 46.96% +6.81%
- Complexity 5927 6162 +235
=============================================
Files 670 681 +11
Lines 20967 21528 +561
=============================================
+ Hits 8418 10110 +1692
+ Misses 12549 11418 -1131
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
This is my Introduction Task and I changed the ProjectController.php and project_details.html.twig to add a steal button. I changed the codeView Button to the steal button.
|
@schaubes ready for review :) |
dc968e3
to
a853379
Compare
This is my Introduction Task and I changed the ProjectController.php and project_details.html.twig to add a steal button. I changed the codeView Button to the steal button.
Your checklist for this pull request
Please review the contributing guidelines and wiki pages of this repository.
SHARE-666 The devils ticketCode Reviewsection in JiraAdditional Description
TODO: Add additional information that is not in your commit-message hereTests - additional information
The tests fail because of the missing code view button
TODO: add additional information about testruns here