From ad864787a749a26a994192225faa1f8959ee8dee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 2 Jun 2020 16:41:59 +0200 Subject: [PATCH] Refactor trailers Trailes are no persons, they have a person. --- .../scm/api/v2/resources/ChangesetDto.java | 4 +-- ...{TrailerPersonDto.java => TrailerDto.java} | 3 ++- .../scm/repository/ChangesetTrailers.java | 1 - .../java/sonia/scm/repository/Trailer.java | 3 +-- scm-ui/ui-types/src/Changesets.ts | 8 +++--- .../changesets/ChangesetDetails.tsx | 26 +++++++++---------- .../ChangesetDescriptionTrailers.java | 3 ++- .../DefaultChangesetToChangesetDtoMapper.java | 11 ++++---- .../v2/resources/BranchRootResourceTest.java | 13 +++++----- .../ChangesetDescriptionTrailersTest.java | 16 ++++++------ 10 files changed, 43 insertions(+), 45 deletions(-) rename scm-core/src/main/java/sonia/scm/api/v2/resources/{TrailerPersonDto.java => TrailerDto.java} (96%) diff --git a/scm-core/src/main/java/sonia/scm/api/v2/resources/ChangesetDto.java b/scm-core/src/main/java/sonia/scm/api/v2/resources/ChangesetDto.java index 08517c2ae1..5b57fa2752 100644 --- a/scm-core/src/main/java/sonia/scm/api/v2/resources/ChangesetDto.java +++ b/scm-core/src/main/java/sonia/scm/api/v2/resources/ChangesetDto.java @@ -21,7 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.api.v2.resources; import de.otto.edison.hal.Embedded; @@ -59,7 +59,7 @@ public class ChangesetDto extends HalRepresentation { */ private String description; - private List trailerPersons; + private List trailers; public ChangesetDto(Links links, Embedded embedded) { super(links, embedded); diff --git a/scm-core/src/main/java/sonia/scm/api/v2/resources/TrailerPersonDto.java b/scm-core/src/main/java/sonia/scm/api/v2/resources/TrailerDto.java similarity index 96% rename from scm-core/src/main/java/sonia/scm/api/v2/resources/TrailerPersonDto.java rename to scm-core/src/main/java/sonia/scm/api/v2/resources/TrailerDto.java index ecf09c3552..2d1b65c6c5 100644 --- a/scm-core/src/main/java/sonia/scm/api/v2/resources/TrailerPersonDto.java +++ b/scm-core/src/main/java/sonia/scm/api/v2/resources/TrailerDto.java @@ -31,6 +31,7 @@ import lombok.Setter; @Getter @Setter @NoArgsConstructor -public class TrailerPersonDto extends PersonDto { +public class TrailerDto { private String trailerType; + private PersonDto person; } diff --git a/scm-core/src/main/java/sonia/scm/repository/ChangesetTrailers.java b/scm-core/src/main/java/sonia/scm/repository/ChangesetTrailers.java index 984a3d6e07..d8ae8eb6db 100644 --- a/scm-core/src/main/java/sonia/scm/repository/ChangesetTrailers.java +++ b/scm-core/src/main/java/sonia/scm/repository/ChangesetTrailers.java @@ -24,7 +24,6 @@ package sonia.scm.repository; -import sonia.scm.api.v2.resources.TrailerPersonDto; import sonia.scm.plugin.ExtensionPoint; import java.util.List; diff --git a/scm-core/src/main/java/sonia/scm/repository/Trailer.java b/scm-core/src/main/java/sonia/scm/repository/Trailer.java index 9dcac3aa67..15bb600b99 100644 --- a/scm-core/src/main/java/sonia/scm/repository/Trailer.java +++ b/scm-core/src/main/java/sonia/scm/repository/Trailer.java @@ -29,6 +29,5 @@ import lombok.Value; @Value public class Trailer { private String trailerType; - private String mail; - private String name; + private Person person; } diff --git a/scm-ui/ui-types/src/Changesets.ts b/scm-ui/ui-types/src/Changesets.ts index 53c4aa53c1..707f716167 100644 --- a/scm-ui/ui-types/src/Changesets.ts +++ b/scm-ui/ui-types/src/Changesets.ts @@ -25,6 +25,7 @@ import { Collection, Links } from "./hal"; import { Tag } from "./Tags"; import { Branch } from "./Branches"; +import { Person } from "@scm-manager/ui-components/src/avatar/Avatar"; export type Changeset = Collection & { id: string; @@ -34,7 +35,7 @@ export type Changeset = Collection & { mail?: string; }; description: string; - trailerPersons: TrailerPerson[]; + trailers: Trailer[]; _links: Links; _embedded: { tags?: Tag[]; @@ -43,9 +44,8 @@ export type Changeset = Collection & { }; }; -export type TrailerPerson = { - name: string; - mail: string; +export type Trailer = { + person: Person; trailerType: string; }; diff --git a/scm-ui/ui-webapp/src/repos/components/changesets/ChangesetDetails.tsx b/scm-ui/ui-webapp/src/repos/components/changesets/ChangesetDetails.tsx index 0b3172642d..84f506749b 100644 --- a/scm-ui/ui-webapp/src/repos/components/changesets/ChangesetDetails.tsx +++ b/scm-ui/ui-webapp/src/repos/components/changesets/ChangesetDetails.tsx @@ -38,7 +38,6 @@ import { DateFromNow, Level } from "@scm-manager/ui-components"; -import { TrailerPerson } from "@scm-manager/ui-types/src/Changesets"; type Props = WithTranslation & { changeset: Changeset; @@ -79,23 +78,22 @@ class ChangesetDetails extends React.Component { const { changeset } = this.props; // @ts-ignore - return [...new Set(changeset.trailerPersons.map(person => person.trailerType))]; + return [...new Set(changeset.trailers.map(trailer => trailer.trailerType))]; } - getTrailersByType(type: string) { + getPersonsByTrailersType(type: string) { const { changeset } = this.props; - return changeset.trailerPersons?.filter(person => person.trailerType === type); + return changeset.trailers?.filter(trailer => trailer.trailerType === type).map(t => t.person); } - getTrailerPersonsByType() { + getTrailersByType() { const availableTrailerTypes: string[] = this.collectAvailableTrailerTypes(); - let type; - let trailerPersons = []; - for (type of availableTrailerTypes) { - trailerPersons.push({ type, persons: this.getTrailersByType(type) }); + const personsByTrailerType = []; + for (const type of availableTrailerTypes) { + personsByTrailerType.push({ type, persons: this.getPersonsByTrailersType(type) }); } - return trailerPersons; + return personsByTrailerType; } render() { @@ -106,7 +104,7 @@ class ChangesetDetails extends React.Component { const id = ; const date = ; - const trailerPersons = this.getTrailerPersonsByType(); + const trailersByType = this.getTrailersByType(); const trailerTable = ( @@ -118,11 +116,11 @@ class ChangesetDetails extends React.Component { - {trailerPersons.map(p => ( + {trailersByType.map(trailer => ( - {t("changeset.trailer.type." + p.type) + ":"} + {t("changeset.trailer.type." + trailer.type) + ":"}
- {p.persons + {trailer.persons .map(person => ( {person.name} diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ChangesetDescriptionTrailers.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ChangesetDescriptionTrailers.java index 0885176f79..7733aaafba 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ChangesetDescriptionTrailers.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ChangesetDescriptionTrailers.java @@ -28,6 +28,7 @@ import com.google.common.collect.ImmutableSet; import sonia.scm.plugin.Extension; import sonia.scm.repository.Changeset; import sonia.scm.repository.ChangesetTrailers; +import sonia.scm.repository.Person; import sonia.scm.repository.Repository; import sonia.scm.repository.Trailer; @@ -79,7 +80,7 @@ public class ChangesetDescriptionTrailers implements ChangesetTrailers { Matcher matcher = PERSON_PATTERN.matcher(person.trim()); if (matcher.matches()) { MatchResult matchResult = matcher.toMatchResult(); - return of(new Trailer(type, matchResult.group(2), matchResult.group(1))); + return of(new Trailer(type, new Person(matchResult.group(1), matchResult.group(2)))); } else { return empty(); } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DefaultChangesetToChangesetDtoMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DefaultChangesetToChangesetDtoMapper.java index 8558dc8a75..7cb5009246 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DefaultChangesetToChangesetDtoMapper.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DefaultChangesetToChangesetDtoMapper.java @@ -29,12 +29,12 @@ import de.otto.edison.hal.Links; import org.mapstruct.AfterMapping; import org.mapstruct.Context; import org.mapstruct.Mapper; -import org.mapstruct.Mapping; import org.mapstruct.MappingTarget; import org.mapstruct.ObjectFactory; import sonia.scm.repository.Branch; import sonia.scm.repository.Changeset; import sonia.scm.repository.ChangesetTrailers; +import sonia.scm.repository.Person; import sonia.scm.repository.Repository; import sonia.scm.repository.Tag; import sonia.scm.repository.Trailer; @@ -77,19 +77,18 @@ public abstract class DefaultChangesetToChangesetDtoMapper extends HalAppenderMa @Inject private Set changesetTrailersSet; -// @Mapping(target = "attributes", ignore = true) // We do not map HAL attributes -// public abstract ChangesetDto map(Changeset changeset, @Context Repository repository); + abstract TrailerDto map(Trailer trailer); - abstract TrailerPersonDto map(Trailer trailer); + abstract PersonDto map(Person person); @AfterMapping void appendTrailerPersons(Changeset changeset, @MappingTarget ChangesetDto target, @Context Repository repository) { - List collectedTrailers = new ArrayList<>(); + List collectedTrailers = new ArrayList<>(); changesetTrailersSet.stream() .flatMap(changesetTrailers -> changesetTrailers.getTrailers(repository, changeset).stream()) .map(this::map) .forEach(collectedTrailers::add); - target.setTrailerPersons(collectedTrailers); + target.setTrailers(collectedTrailers); } @AfterMapping diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/BranchRootResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/BranchRootResourceTest.java index ad4dc0615f..42756108ce 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/BranchRootResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/BranchRootResourceTest.java @@ -21,7 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.api.v2.resources; import com.google.inject.util.Providers; @@ -64,6 +64,7 @@ import java.util.Date; import java.util.List; import java.util.Set; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; @@ -161,7 +162,7 @@ public class BranchRootResourceTest extends RepositoryTestBase { assertEquals(404, response.getStatus()); MediaType contentType = (MediaType) response.getOutputHeaders().getFirst("Content-Type"); - Assertions.assertThat(response.getContentAsString()).contains("branch", "master", "space/repo"); + assertThat(response.getContentAsString()).contains("branch", "master", "space/repo"); } @Test @@ -201,10 +202,10 @@ public class BranchRootResourceTest extends RepositoryTestBase { dispatcher.invoke(request, response); assertEquals(200, response.getStatus()); log.info("Response :{}", response.getContentAsString()); - assertTrue(response.getContentAsString().contains(String.format("\"id\":\"%s\"", REVISION))); - assertTrue(response.getContentAsString().contains(String.format("\"name\":\"%s\"", authorName))); - assertTrue(response.getContentAsString().contains(String.format("\"mail\":\"%s\"", authorEmail))); - assertTrue(response.getContentAsString().contains(String.format("\"description\":\"%s\"", commit))); + assertThat(response.getContentAsString()).contains(String.format("\"id\":\"%s\"", REVISION)); + assertThat(response.getContentAsString()).contains(String.format("\"name\":\"%s\"", authorName)); + assertThat(response.getContentAsString()).contains(String.format("\"mail\":\"%s\"", authorEmail)); + assertThat(response.getContentAsString()).contains(String.format("\"description\":\"%s\"", commit)); } @Test diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ChangesetDescriptionTrailersTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ChangesetDescriptionTrailersTest.java index 84cf2d0574..d1aebd592e 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ChangesetDescriptionTrailersTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ChangesetDescriptionTrailersTest.java @@ -64,8 +64,8 @@ class ChangesetDescriptionTrailersTest { Trailer first = Trailer.get(0); assertThat(first.getTrailerType()).isEqualTo("Co-authored-by"); - assertThat(first.getName()).isEqualTo(displayUser.getDisplayName()); - assertThat(first.getMail()).isEqualTo(displayUser.getMail()); + assertThat(first.getPerson().getName()).isEqualTo(displayUser.getDisplayName()); + assertThat(first.getPerson().getMail()).isEqualTo(displayUser.getMail()); } @Test @@ -78,8 +78,8 @@ class ChangesetDescriptionTrailersTest { Trailer Trailer = trailer.get(0); assertThat(Trailer.getTrailerType()).isEqualTo("Reviewed-by"); - assertThat(Trailer.getName()).isEqualTo(displayUser.getDisplayName()); - assertThat(Trailer.getMail()).isEqualTo(displayUser.getMail()); + assertThat(Trailer.getPerson().getName()).isEqualTo(displayUser.getDisplayName()); + assertThat(Trailer.getPerson().getMail()).isEqualTo(displayUser.getMail()); } @Test @@ -92,8 +92,8 @@ class ChangesetDescriptionTrailersTest { Trailer Trailer = trailer.get(0); assertThat(Trailer.getTrailerType()).isEqualTo("Signed-off-by"); - assertThat(Trailer.getName()).isEqualTo(displayUser.getDisplayName()); - assertThat(Trailer.getMail()).isEqualTo(displayUser.getMail()); + assertThat(Trailer.getPerson().getName()).isEqualTo(displayUser.getDisplayName()); + assertThat(Trailer.getPerson().getMail()).isEqualTo(displayUser.getMail()); } @Test @@ -106,8 +106,8 @@ class ChangesetDescriptionTrailersTest { Trailer Trailer = trailer.get(0); assertThat(Trailer.getTrailerType()).isEqualTo("Committed-by"); - assertThat(Trailer.getName()).isEqualTo(displayUser.getDisplayName()); - assertThat(Trailer.getMail()).isEqualTo(displayUser.getMail()); + assertThat(Trailer.getPerson().getName()).isEqualTo(displayUser.getDisplayName()); + assertThat(Trailer.getPerson().getMail()).isEqualTo(displayUser.getMail()); } private Changeset createChangeset(String commitMessage) {