Refactor trailers

Trailes are no persons, they have a person.
This commit is contained in:
René Pfeuffer
2020-06-02 16:41:59 +02:00
parent 76f2722ff4
commit ad864787a7
10 changed files with 43 additions and 45 deletions

View File

@@ -21,7 +21,7 @@
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
* SOFTWARE. * SOFTWARE.
*/ */
package sonia.scm.api.v2.resources; package sonia.scm.api.v2.resources;
import de.otto.edison.hal.Embedded; import de.otto.edison.hal.Embedded;
@@ -59,7 +59,7 @@ public class ChangesetDto extends HalRepresentation {
*/ */
private String description; private String description;
private List<TrailerPersonDto> trailerPersons; private List<TrailerDto> trailers;
public ChangesetDto(Links links, Embedded embedded) { public ChangesetDto(Links links, Embedded embedded) {
super(links, embedded); super(links, embedded);

View File

@@ -31,6 +31,7 @@ import lombok.Setter;
@Getter @Getter
@Setter @Setter
@NoArgsConstructor @NoArgsConstructor
public class TrailerPersonDto extends PersonDto { public class TrailerDto {
private String trailerType; private String trailerType;
private PersonDto person;
} }

View File

@@ -24,7 +24,6 @@
package sonia.scm.repository; package sonia.scm.repository;
import sonia.scm.api.v2.resources.TrailerPersonDto;
import sonia.scm.plugin.ExtensionPoint; import sonia.scm.plugin.ExtensionPoint;
import java.util.List; import java.util.List;

View File

@@ -29,6 +29,5 @@ import lombok.Value;
@Value @Value
public class Trailer { public class Trailer {
private String trailerType; private String trailerType;
private String mail; private Person person;
private String name;
} }

View File

@@ -25,6 +25,7 @@
import { Collection, Links } from "./hal"; import { Collection, Links } from "./hal";
import { Tag } from "./Tags"; import { Tag } from "./Tags";
import { Branch } from "./Branches"; import { Branch } from "./Branches";
import { Person } from "@scm-manager/ui-components/src/avatar/Avatar";
export type Changeset = Collection & { export type Changeset = Collection & {
id: string; id: string;
@@ -34,7 +35,7 @@ export type Changeset = Collection & {
mail?: string; mail?: string;
}; };
description: string; description: string;
trailerPersons: TrailerPerson[]; trailers: Trailer[];
_links: Links; _links: Links;
_embedded: { _embedded: {
tags?: Tag[]; tags?: Tag[];
@@ -43,9 +44,8 @@ export type Changeset = Collection & {
}; };
}; };
export type TrailerPerson = { export type Trailer = {
name: string; person: Person;
mail: string;
trailerType: string; trailerType: string;
}; };

View File

@@ -38,7 +38,6 @@ import {
DateFromNow, DateFromNow,
Level Level
} from "@scm-manager/ui-components"; } from "@scm-manager/ui-components";
import { TrailerPerson } from "@scm-manager/ui-types/src/Changesets";
type Props = WithTranslation & { type Props = WithTranslation & {
changeset: Changeset; changeset: Changeset;
@@ -79,23 +78,22 @@ class ChangesetDetails extends React.Component<Props, State> {
const { changeset } = this.props; const { changeset } = this.props;
// @ts-ignore // @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; 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(); const availableTrailerTypes: string[] = this.collectAvailableTrailerTypes();
let type; const personsByTrailerType = [];
let trailerPersons = []; for (const type of availableTrailerTypes) {
for (type of availableTrailerTypes) { personsByTrailerType.push({ type, persons: this.getPersonsByTrailersType(type) });
trailerPersons.push({ type, persons: this.getTrailersByType(type) });
} }
return trailerPersons; return personsByTrailerType;
} }
render() { render() {
@@ -106,7 +104,7 @@ class ChangesetDetails extends React.Component<Props, State> {
const id = <ChangesetId repository={repository} changeset={changeset} link={false} />; const id = <ChangesetId repository={repository} changeset={changeset} link={false} />;
const date = <DateFromNow date={changeset.date} />; const date = <DateFromNow date={changeset.date} />;
const trailerPersons = this.getTrailerPersonsByType(); const trailersByType = this.getTrailersByType();
const trailerTable = ( const trailerTable = (
<table> <table>
@@ -118,11 +116,11 @@ class ChangesetDetails extends React.Component<Props, State> {
</a> </a>
</td> </td>
</tr> </tr>
{trailerPersons.map(p => ( {trailersByType.map(trailer => (
<tr> <tr>
<SizedTd>{t("changeset.trailer.type." + p.type) + ":"}</SizedTd> <SizedTd>{t("changeset.trailer.type." + trailer.type) + ":"}</SizedTd>
<td className="shorten-text is-marginless"> <td className="shorten-text is-marginless">
{p.persons {trailer.persons
.map(person => ( .map(person => (
<a title={person.mail} href={"mailto:" + person.mail}> <a title={person.mail} href={"mailto:" + person.mail}>
{person.name} {person.name}

View File

@@ -28,6 +28,7 @@ import com.google.common.collect.ImmutableSet;
import sonia.scm.plugin.Extension; import sonia.scm.plugin.Extension;
import sonia.scm.repository.Changeset; import sonia.scm.repository.Changeset;
import sonia.scm.repository.ChangesetTrailers; import sonia.scm.repository.ChangesetTrailers;
import sonia.scm.repository.Person;
import sonia.scm.repository.Repository; import sonia.scm.repository.Repository;
import sonia.scm.repository.Trailer; import sonia.scm.repository.Trailer;
@@ -79,7 +80,7 @@ public class ChangesetDescriptionTrailers implements ChangesetTrailers {
Matcher matcher = PERSON_PATTERN.matcher(person.trim()); Matcher matcher = PERSON_PATTERN.matcher(person.trim());
if (matcher.matches()) { if (matcher.matches()) {
MatchResult matchResult = matcher.toMatchResult(); 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 { } else {
return empty(); return empty();
} }

View File

@@ -29,12 +29,12 @@ import de.otto.edison.hal.Links;
import org.mapstruct.AfterMapping; import org.mapstruct.AfterMapping;
import org.mapstruct.Context; import org.mapstruct.Context;
import org.mapstruct.Mapper; import org.mapstruct.Mapper;
import org.mapstruct.Mapping;
import org.mapstruct.MappingTarget; import org.mapstruct.MappingTarget;
import org.mapstruct.ObjectFactory; import org.mapstruct.ObjectFactory;
import sonia.scm.repository.Branch; import sonia.scm.repository.Branch;
import sonia.scm.repository.Changeset; import sonia.scm.repository.Changeset;
import sonia.scm.repository.ChangesetTrailers; import sonia.scm.repository.ChangesetTrailers;
import sonia.scm.repository.Person;
import sonia.scm.repository.Repository; import sonia.scm.repository.Repository;
import sonia.scm.repository.Tag; import sonia.scm.repository.Tag;
import sonia.scm.repository.Trailer; import sonia.scm.repository.Trailer;
@@ -77,19 +77,18 @@ public abstract class DefaultChangesetToChangesetDtoMapper extends HalAppenderMa
@Inject @Inject
private Set<ChangesetTrailers> changesetTrailersSet; private Set<ChangesetTrailers> changesetTrailersSet;
// @Mapping(target = "attributes", ignore = true) // We do not map HAL attributes abstract TrailerDto map(Trailer trailer);
// public abstract ChangesetDto map(Changeset changeset, @Context Repository repository);
abstract TrailerPersonDto map(Trailer trailer); abstract PersonDto map(Person person);
@AfterMapping @AfterMapping
void appendTrailerPersons(Changeset changeset, @MappingTarget ChangesetDto target, @Context Repository repository) { void appendTrailerPersons(Changeset changeset, @MappingTarget ChangesetDto target, @Context Repository repository) {
List<TrailerPersonDto> collectedTrailers = new ArrayList<>(); List<TrailerDto> collectedTrailers = new ArrayList<>();
changesetTrailersSet.stream() changesetTrailersSet.stream()
.flatMap(changesetTrailers -> changesetTrailers.getTrailers(repository, changeset).stream()) .flatMap(changesetTrailers -> changesetTrailers.getTrailers(repository, changeset).stream())
.map(this::map) .map(this::map)
.forEach(collectedTrailers::add); .forEach(collectedTrailers::add);
target.setTrailerPersons(collectedTrailers); target.setTrailers(collectedTrailers);
} }
@AfterMapping @AfterMapping

View File

@@ -21,7 +21,7 @@
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
* SOFTWARE. * SOFTWARE.
*/ */
package sonia.scm.api.v2.resources; package sonia.scm.api.v2.resources;
import com.google.inject.util.Providers; import com.google.inject.util.Providers;
@@ -64,6 +64,7 @@ import java.util.Date;
import java.util.List; import java.util.List;
import java.util.Set; import java.util.Set;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.any;
@@ -161,7 +162,7 @@ public class BranchRootResourceTest extends RepositoryTestBase {
assertEquals(404, response.getStatus()); assertEquals(404, response.getStatus());
MediaType contentType = (MediaType) response.getOutputHeaders().getFirst("Content-Type"); 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 @Test
@@ -201,10 +202,10 @@ public class BranchRootResourceTest extends RepositoryTestBase {
dispatcher.invoke(request, response); dispatcher.invoke(request, response);
assertEquals(200, response.getStatus()); assertEquals(200, response.getStatus());
log.info("Response :{}", response.getContentAsString()); log.info("Response :{}", response.getContentAsString());
assertTrue(response.getContentAsString().contains(String.format("\"id\":\"%s\"", REVISION))); assertThat(response.getContentAsString()).contains(String.format("\"id\":\"%s\"", REVISION));
assertTrue(response.getContentAsString().contains(String.format("\"name\":\"%s\"", authorName))); assertThat(response.getContentAsString()).contains(String.format("\"name\":\"%s\"", authorName));
assertTrue(response.getContentAsString().contains(String.format("\"mail\":\"%s\"", authorEmail))); assertThat(response.getContentAsString()).contains(String.format("\"mail\":\"%s\"", authorEmail));
assertTrue(response.getContentAsString().contains(String.format("\"description\":\"%s\"", commit))); assertThat(response.getContentAsString()).contains(String.format("\"description\":\"%s\"", commit));
} }
@Test @Test

View File

@@ -64,8 +64,8 @@ class ChangesetDescriptionTrailersTest {
Trailer first = Trailer.get(0); Trailer first = Trailer.get(0);
assertThat(first.getTrailerType()).isEqualTo("Co-authored-by"); assertThat(first.getTrailerType()).isEqualTo("Co-authored-by");
assertThat(first.getName()).isEqualTo(displayUser.getDisplayName()); assertThat(first.getPerson().getName()).isEqualTo(displayUser.getDisplayName());
assertThat(first.getMail()).isEqualTo(displayUser.getMail()); assertThat(first.getPerson().getMail()).isEqualTo(displayUser.getMail());
} }
@Test @Test
@@ -78,8 +78,8 @@ class ChangesetDescriptionTrailersTest {
Trailer Trailer = trailer.get(0); Trailer Trailer = trailer.get(0);
assertThat(Trailer.getTrailerType()).isEqualTo("Reviewed-by"); assertThat(Trailer.getTrailerType()).isEqualTo("Reviewed-by");
assertThat(Trailer.getName()).isEqualTo(displayUser.getDisplayName()); assertThat(Trailer.getPerson().getName()).isEqualTo(displayUser.getDisplayName());
assertThat(Trailer.getMail()).isEqualTo(displayUser.getMail()); assertThat(Trailer.getPerson().getMail()).isEqualTo(displayUser.getMail());
} }
@Test @Test
@@ -92,8 +92,8 @@ class ChangesetDescriptionTrailersTest {
Trailer Trailer = trailer.get(0); Trailer Trailer = trailer.get(0);
assertThat(Trailer.getTrailerType()).isEqualTo("Signed-off-by"); assertThat(Trailer.getTrailerType()).isEqualTo("Signed-off-by");
assertThat(Trailer.getName()).isEqualTo(displayUser.getDisplayName()); assertThat(Trailer.getPerson().getName()).isEqualTo(displayUser.getDisplayName());
assertThat(Trailer.getMail()).isEqualTo(displayUser.getMail()); assertThat(Trailer.getPerson().getMail()).isEqualTo(displayUser.getMail());
} }
@Test @Test
@@ -106,8 +106,8 @@ class ChangesetDescriptionTrailersTest {
Trailer Trailer = trailer.get(0); Trailer Trailer = trailer.get(0);
assertThat(Trailer.getTrailerType()).isEqualTo("Committed-by"); assertThat(Trailer.getTrailerType()).isEqualTo("Committed-by");
assertThat(Trailer.getName()).isEqualTo(displayUser.getDisplayName()); assertThat(Trailer.getPerson().getName()).isEqualTo(displayUser.getDisplayName());
assertThat(Trailer.getMail()).isEqualTo(displayUser.getMail()); assertThat(Trailer.getPerson().getMail()).isEqualTo(displayUser.getMail());
} }
private Changeset createChangeset(String commitMessage) { private Changeset createChangeset(String commitMessage) {