Added project missing fields #178

BrunoRosendo · 2023-07-20T00:58:20Z

Review checklist

Properly documents API changes in the corresponding controller test
Contains enough appropriate tests
Behavior is as expected
Clean, well structured code

src/main/kotlin/pt/up/fe/ni/website/backend/service/activity/EventService.kt

BrunoRosendo · 2023-07-22T02:18:53Z

src/main/kotlin/pt/up/fe/ni/website/backend/controller/ProjectController.kt

+    @PutMapping("/{id}", consumes = ["multipart/form-data"])
    fun updateProjectById(
        @PathVariable id: Long,
-        @RequestBody dto: ProjectDto
-    ) = service.updateProjectById(id, dto)
+        @RequestPart project: ProjectDto,
+        @RequestParam
+        @ValidImage
+        image: MultipartFile?
+    ): Project {
+        project.imageFile = image
+        return service.updateProjectById(id, project)
+    }


In this version, the way to update the timeline is to send a PUT with the whole project, the same way we do for the remaining fields. I think this makes the API simpler and is fine since the timeline doesn't hold a lot of data

github-actions · 2023-07-22T02:22:51Z

Check the documentation preview: https://64bb3d784bb2041328a940f5--niaefeup-backend-docs.netlify.app

github-actions · 2023-07-25T17:29:33Z

Check the documentation preview: https://64c0067aad4acc0093b0c053--niaefeup-backend-docs.netlify.app

MRita443

Awesome job! Some great much needed generalizations besides the additions here. Left mostly questions

src/main/kotlin/pt/up/fe/ni/website/backend/model/Project.kt

src/main/kotlin/pt/up/fe/ni/website/backend/controller/AccountController.kt

src/main/kotlin/pt/up/fe/ni/website/backend/controller/ErrorController.kt

src/main/kotlin/pt/up/fe/ni/website/backend/controller/EventController.kt

src/main/kotlin/pt/up/fe/ni/website/backend/dto/entity/EntityDto.kt

src/main/kotlin/pt/up/fe/ni/website/backend/service/activity/AbstractActivityService.kt

...tlin/pt/up/fe/ni/website/backend/utils/documentation/payloadschemas/model/PayloadActivity.kt

src/test/kotlin/pt/up/fe/ni/website/backend/controller/AccountControllerTest.kt

src/test/kotlin/pt/up/fe/ni/website/backend/controller/EventControllerTest.kt

MRita443

Some other minor questions here and there. Also not sure why the test action is failing

src/test/kotlin/pt/up/fe/ni/website/backend/controller/AccountControllerTest.kt

src/test/kotlin/pt/up/fe/ni/website/backend/controller/ProjectControllerTest.kt

...otlin/pt/up/fe/ni/website/backend/utils/documentation/payloadschemas/model/PayloadProject.kt

jamcunha

This PR appears to be almost ready. I left just a few questions.

src/main/resources/static/events/.gitkeep

src/test/kotlin/pt/up/fe/ni/website/backend/controller/AccountControllerTest.kt

BrunoRosendo · 2023-08-02T02:04:32Z

I have no idea as to why the build is failing here with a compilation error. Everything works normally locally and the error does not make sense to me😕

MRita443 · 2023-08-02T14:14:52Z

I have no idea as to why the build is failing here with a compilation error. Everything works normally locally and the error does not make sense to me😕

Everything worked for me locally too, it's weird but imo the most important thing is that it works locally and when it's ready we can merge it anyways

limwa · 2023-08-02T14:20:49Z

I have no idea as to why the build is failing here with a compilation error. Everything works normally locally and the error does not make sense to me😕

Everything worked for me locally too, it's weird but imo the most important thing is that it works locally and when it's ready we can merge it anyways

I'll look into it as well. I don't think it's wise to merge until we know what is happening.

github-actions · 2023-08-07T18:44:19Z

Check the documentation preview: https://64d13b7e3f5d6a15627613ce--niaefeup-backend-docs.netlify.app

github-actions · 2023-08-07T19:01:51Z

Check the documentation preview: https://64d13f9cbb9f331953df344e--niaefeup-backend-docs.netlify.app

MRita443

Great job as usual :)

MRita443 · 2023-08-08T15:18:46Z

src/main/kotlin/pt/up/fe/ni/website/backend/service/activity/ProjectService.kt

        newProject.apply {
-            teamMembers.clear()
-            dto.teamMembersIds?.forEach {
+            hallOfFame.clear()


Good catch here!

jamcunha

Good job!

github-actions · 2023-08-08T16:17:40Z

Check the documentation preview:

BrunoRosendo mentioned this pull request Jul 21, 2023

docs: documentation for multi part form #144

Open

BrunoRosendo commented Jul 22, 2023

View reviewed changes

BrunoRosendo linked an issue Jul 22, 2023 that may be closed by this pull request

projects: timeline #65

Closed

BrunoRosendo marked this pull request as ready for review July 25, 2023 17:25

BrunoRosendo requested review from MRita443, jamcunha and limwa July 25, 2023 17:26

MRita443 requested changes Jul 30, 2023

View reviewed changes

BrunoRosendo force-pushed the feature/project-missing-fields branch from ba351fb to 88ce5bf Compare July 31, 2023 23:42

BrunoRosendo requested a review from MRita443 July 31, 2023 23:42

BrunoRosendo force-pushed the feature/project-missing-fields branch 2 times, most recently from 5dab80d to a8175f7 Compare August 1, 2023 00:01

MRita443 requested changes Aug 1, 2023

View reviewed changes

BrunoRosendo force-pushed the feature/project-missing-fields branch from 74cd6c1 to 247fa9a Compare August 1, 2023 22:12

jamcunha reviewed Aug 1, 2023

View reviewed changes

src/main/resources/static/events/.gitkeep Outdated Show resolved Hide resolved

src/test/kotlin/pt/up/fe/ni/website/backend/controller/AccountControllerTest.kt Outdated Show resolved Hide resolved

BrunoRosendo force-pushed the feature/project-missing-fields branch 7 times, most recently from 8d316a0 to 55eb68f Compare August 2, 2023 02:03

BrunoRosendo requested review from jamcunha and MRita443 August 2, 2023 02:03

BrunoRosendo force-pushed the feature/project-missing-fields branch from 55eb68f to 2024c42 Compare August 2, 2023 02:06

BrunoRosendo added 16 commits August 7, 2023 19:02

Added missing test to account controller

2ed7f19

Added documentation for new activity fields

2130a66

Added tests when account part is missing

0ad9ce8

Updated event controller tests

be5ea76

Updated project controller tests

66a20e0

Testing team updates in PUT methods

7b1a665

Refactored activity services to avoid code duplication

ae787d9

Added github field to projects

bd710ff

Comparing activity slugs first to avoid fetch

7cb9ec9

Refactored account test mocks

8bede0c

Fixed wrong optional fields in project payload

8b41290

Removed wildcard imports

a3449e9

Creating upload directory if it doesn't exist

5fcb686

Added webp file type

c8e6c5d

Fixed malformed account test

a586226

Replaced wildcard import

8027707

BrunoRosendo force-pushed the feature/project-missing-fields branch from 218b693 to 8762094 Compare August 7, 2023 18:26

BrunoRosendo requested a review from MRita443 August 7, 2023 18:26

BrunoRosendo force-pushed the feature/project-missing-fields branch from 8762094 to 0fdae7e Compare August 7, 2023 18:28

Fixed wrong team in project test

511e498

BrunoRosendo force-pushed the feature/project-missing-fields branch from 0fdae7e to 511e498 Compare August 7, 2023 18:38

Updating hall of fame when updating projects

05a4181

BrunoRosendo force-pushed the feature/project-missing-fields branch from c6f6983 to 05a4181 Compare August 7, 2023 18:56

MRita443 approved these changes Aug 8, 2023

View reviewed changes

jamcunha approved these changes Aug 8, 2023

View reviewed changes

BrunoRosendo merged commit 8f44a36 into develop Aug 8, 2023
3 checks passed

BrunoRosendo deleted the feature/project-missing-fields branch August 8, 2023 16:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added project missing fields #178

Added project missing fields #178

BrunoRosendo commented Jul 20, 2023

BrunoRosendo Jul 22, 2023

github-actions bot commented Jul 22, 2023

github-actions bot commented Jul 25, 2023

MRita443 left a comment

MRita443 left a comment

jamcunha left a comment

BrunoRosendo commented Aug 2, 2023 •

edited

Loading

MRita443 commented Aug 2, 2023

limwa commented Aug 2, 2023

github-actions bot commented Aug 7, 2023

github-actions bot commented Aug 7, 2023

MRita443 left a comment

MRita443 Aug 8, 2023

jamcunha left a comment

github-actions bot commented Aug 8, 2023

Added project missing fields #178

Added project missing fields #178

Conversation

BrunoRosendo commented Jul 20, 2023

Review checklist

BrunoRosendo Jul 22, 2023

Choose a reason for hiding this comment

github-actions bot commented Jul 22, 2023

github-actions bot commented Jul 25, 2023

MRita443 left a comment

Choose a reason for hiding this comment

MRita443 left a comment

Choose a reason for hiding this comment

jamcunha left a comment

Choose a reason for hiding this comment

BrunoRosendo commented Aug 2, 2023 • edited Loading

MRita443 commented Aug 2, 2023

limwa commented Aug 2, 2023

github-actions bot commented Aug 7, 2023

github-actions bot commented Aug 7, 2023

MRita443 left a comment

Choose a reason for hiding this comment

MRita443 Aug 8, 2023

Choose a reason for hiding this comment

jamcunha left a comment

Choose a reason for hiding this comment

github-actions bot commented Aug 8, 2023

BrunoRosendo commented Aug 2, 2023 •

edited

Loading