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.

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

fix avatar initials for usernames with whitespace #971

Merged
merged 5 commits into from
Nov 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,15 @@
import org.springframework.http.CacheControl;
import org.springframework.http.MediaType;
import org.springframework.http.ResponseEntity;
import org.springframework.stereotype.Controller;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.ResponseBody;
import org.springframework.web.bind.annotation.RestController;

import java.util.Locale;
import java.util.Map;
import java.util.concurrent.TimeUnit;

@Controller
@RestController
public class AvatarController {

private final SvgService svgService;
Expand All @@ -22,7 +21,6 @@ public class AvatarController {
}

@GetMapping(value = "/avatar", produces = "image/svg+xml")
@ResponseBody
public ResponseEntity<String> avatar(@RequestParam("name") String name, Locale locale) {

final Map<String, Object> model = Map.of("initials", getInitials(name));
Expand All @@ -35,11 +33,14 @@ public ResponseEntity<String> avatar(@RequestParam("name") String name, Locale l
}

private static String getInitials(String niceName) {
final int idxLastWhitespace = niceName.lastIndexOf(' ');

final String normalizedNiceName = niceName.strip();

final int idxLastWhitespace = normalizedNiceName.lastIndexOf(' ');
if (idxLastWhitespace == -1) {
return niceName.substring(0, 1).toUpperCase();
return normalizedNiceName.substring(0, 1).toUpperCase();
}

return (niceName.charAt(0) + niceName.substring(idxLastWhitespace + 1, idxLastWhitespace + 2)).toUpperCase();
return (normalizedNiceName.charAt(0) + normalizedNiceName.substring(idxLastWhitespace + 1, idxLastWhitespace + 2)).toUpperCase();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public TenantUser createNewUser(String uuid, String givenName, String familyName
final Instant now = clock.instant();

final TenantUserEntity tenantUserEntity =
new TenantUserEntity(null, uuid, now, now, givenName, familyName, eMailAddress.value(), distinct(authorities), now, now, null, null, UserStatus.ACTIVE);
new TenantUserEntity(null, uuid, now, now, givenName.strip(), familyName.strip(), eMailAddress.value(), distinct(authorities), now, now, null, null, UserStatus.ACTIVE);

final TenantUserEntity persisted = tenantUserRepository.save(tenantUserEntity);

Expand All @@ -56,8 +56,13 @@ public TenantUser updateUser(TenantUser user) {

final TenantUserEntity current = getTenantUserOrThrow(user.localId());

final String givenName = user.givenName().strip();
final String familyName = user.familyName().strip();
final String email = user.eMail().value();
derTobsch marked this conversation as resolved.
Show resolved Hide resolved
final Set<SecurityRole> authorities = distinct(user.authorities());

final TenantUserEntity next =
new TenantUserEntity(current.getId(), current.getUuid(), current.getFirstLoginAt(), now, user.givenName(), user.familyName(), user.eMail().value(), distinct(user.authorities()), current.getCreatedAt(), now, current.getDeactivatedAt(), current.getDeletedAt(), current.getStatus());
new TenantUserEntity(current.getId(), current.getUuid(), current.getFirstLoginAt(), now, givenName, familyName, email, authorities, current.getCreatedAt(), now, current.getDeactivatedAt(), current.getDeletedAt(), current.getStatus());

final TenantUserEntity persisted = tenantUserRepository.save(next);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?xml version="1.1" encoding="UTF-8" standalone="no"?>
<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.5.xsd">

<changeSet author="seber" id="remove-leading-trailing-whitespaces-given-name">
<preConditions>
<tableExists tableName="tenant_user"/>
<columnExists tableName="tenant_user" columnName="given_name"/>
</preConditions>
<update tableName="tenant_user">
<column name="given_name" value="TRIM(given_name)" />
<where>
given_name LIKE ' %' OR given_name LIKE '% '
</where>
</update>
</changeSet>

<changeSet author="seber" id="remove-leading-trailing-whitespaces-family-name">
<preConditions>
<tableExists tableName="tenant_user"/>
<columnExists tableName="tenant_user" columnName="family_name"/>
</preConditions>
<update tableName="tenant_user">
<column name="family_name" value="TRIM(family_name)" />
<where>
family_name LIKE ' %' OR family_name LIKE '% '
</where>
</update>
</changeSet>

</databaseChangeLog>

1 change: 1 addition & 0 deletions src/main/resources/db/changelog/db.changelog-main.xml
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,6 @@
<include relativeToChangelogFile="true" file="changelog-2.5.0-delete-tenant-user-cascade.xml"/>
<include relativeToChangelogFile="true" file="changelog-2.7.0-add-user-status.xml"/>
<include relativeToChangelogFile="true" file="changelog-2.6.4-enable-row-level-security-on-absence.xml"/>
<include relativeToChangelogFile="true" file="changelog-2.14.0-remove-leading-trailing-whitespaces-username.xml"/>

</databaseChangeLog>
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ void setUp() {
"The Batman,TB",
"Batman,B",
"The ultimate Batman,TB",
"' normal one ',NO",
})
void ensureGeneratesAvatarWithInitials(String name, String expectedInitials) throws Exception {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,28 +30,14 @@
class TenantUserServiceImplTest {

private final Clock clock = Clock.fixed(Instant.now(), ZoneId.systemDefault());

private TenantUserServiceImpl sut;

@Mock
private TenantUserRepository repository;
@Mock
private ApplicationEventPublisher publisher;

private static TenantUser activeTenantUserOne(Instant now) {
return new TenantUser("my-external-id-1", 1L, "batman", "batman", new EMailAddress("[email protected]"), now, Set.of(), now, now, null, null, UserStatus.ACTIVE);
}

private static TenantUserEntity activeUserEntityOne(Instant now) {
return new TenantUserEntity(1L, "my-external-id-1", now, now, "batman", "batman", "[email protected]", Set.of(), now, now, null, null, UserStatus.ACTIVE);
}

private static TenantUser activeTenantUserTwo(Instant now) {
return new TenantUser("my-external-id-2", 2L, "petra", "panter", new EMailAddress("[email protected]"), now, Set.of(), now, now, null, null, UserStatus.ACTIVE);
}

private static TenantUserEntity activeUserEntityTwo(Instant now) {
return new TenantUserEntity(2L, "my-external-id-2", now, now, "petra", "panter", "[email protected]", Set.of(), now, now, null, null, UserStatus.ACTIVE);
}

@BeforeEach
void setUp() {
sut = new TenantUserServiceImpl(repository, publisher, clock);
Expand Down Expand Up @@ -237,6 +223,30 @@ void newUserIsCreatedSuccessfully() {

verify(publisher).publishEvent(any(TenantUserCreatedEvent.class));
}

@Test
void ensureGivenNameAndFamilyNameIsStripped() {

final String uuid = "my-external-id";
final String givenName = " Hans Joachim ";
final String familyName = " Sonnencreme ";
final EMailAddress eMailAddress = new EMailAddress("[email protected]");

when(repository.save(any(TenantUserEntity.class))).thenAnswer(returnsFirstArg());

final TenantUser result = sut.createNewUser(uuid, givenName, familyName, eMailAddress, Set.of());

final ArgumentCaptor<TenantUserEntity> entityArgumentCaptor = ArgumentCaptor.forClass(TenantUserEntity.class);
verify(repository).save(entityArgumentCaptor.capture());

assertThat(entityArgumentCaptor.getValue()).satisfies(entity -> {
assertThat(entity.getGivenName()).isEqualTo("Hans Joachim");
assertThat(entity.getFamilyName()).isEqualTo("Sonnencreme");
});

assertThat(result.givenName()).isEqualTo("Hans Joachim");
assertThat(result.familyName()).isEqualTo("Sonnencreme");
}
}

@Nested
Expand All @@ -258,7 +268,6 @@ void updateUserSuccessfully() {

TenantUser result = sut.updateUser(update);


ArgumentCaptor<TenantUserEntity> entityArgumentCaptor = ArgumentCaptor.forClass(TenantUserEntity.class);

verify(repository).save(entityArgumentCaptor.capture());
Expand Down Expand Up @@ -288,6 +297,31 @@ void updateUserSuccessfully() {
});
}

@Test
void ensureGivenNameAndFamilyNameIsStripped() {

final Instant now = clock.instant();

final TenantUserEntity currentEntity = activeUserEntityOne(now);
final TenantUser update = updateFromEntity(currentEntity, " Hans Joachim ", " Sonnencreme ", "[email protected]");

when(repository.findById(any())).thenReturn(Optional.of(currentEntity));
when(repository.save(any(TenantUserEntity.class))).thenAnswer(returnsFirstArg());

final TenantUser result = sut.updateUser(update);

final ArgumentCaptor<TenantUserEntity> entityArgumentCaptor = ArgumentCaptor.forClass(TenantUserEntity.class);
verify(repository).save(entityArgumentCaptor.capture());

assertThat(entityArgumentCaptor.getValue()).satisfies(entity -> {
assertThat(entity.getGivenName()).isEqualTo("Hans Joachim");
assertThat(entity.getFamilyName()).isEqualTo("Sonnencreme");
});

assertThat(result.givenName()).isEqualTo("Hans Joachim");
assertThat(result.familyName()).isEqualTo("Sonnencreme");
}

@Test
void updateUserThrowsExceptionWhenUserNotFound() {
TenantUser user = activeTenantUserOne(Instant.now());
Expand Down Expand Up @@ -413,8 +447,22 @@ void deactivateUserThrowsExceptionWhenUserNotFound() {

verify(repository).findById(userId);
}

}
}

private static TenantUser activeTenantUserOne(Instant now) {
return new TenantUser("my-external-id-1", 1L, "batman", "batman", new EMailAddress("[email protected]"), now, Set.of(), now, now, null, null, UserStatus.ACTIVE);
}

private static TenantUserEntity activeUserEntityOne(Instant now) {
return new TenantUserEntity(1L, "my-external-id-1", now, now, "batman", "batman", "[email protected]", Set.of(), now, now, null, null, UserStatus.ACTIVE);
}

private static TenantUser activeTenantUserTwo(Instant now) {
return new TenantUser("my-external-id-2", 2L, "petra", "panter", new EMailAddress("[email protected]"), now, Set.of(), now, now, null, null, UserStatus.ACTIVE);
}

private static TenantUserEntity activeUserEntityTwo(Instant now) {
return new TenantUserEntity(2L, "my-external-id-2", now, now, "petra", "panter", "[email protected]", Set.of(), now, now, null, null, UserStatus.ACTIVE);
}
}
Loading