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.

Sign up for GitHub

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

Jump to bottom

Update signup with empty names #299

Merged
merged 17 commits into from
Aug 28, 2024
Merged

Update signup with empty names #299

merged 17 commits into from
Aug 28, 2024

Conversation

uchitsa
Copy link
Contributor

@uchitsa uchitsa commented Aug 15, 2024

No description provided.

@uchitsa uchitsa mentioned this pull request Aug 15, 2024
Closed
@uchitsa
Copy link
Contributor Author

uchitsa commented Aug 15, 2024

@fey @dzencot hi, please check PR

@SibirBear
Copy link
Contributor

@uchitsa, привет!
Проверь свое решение, аннотация @SiZe подразумевает, что размер значения должно быть от 1 и больше, другими словами необходимо все равно указать 1 знак (хоть пробел) чтобы успешно зарегистрироваться.

Кроме того, такой импорт "import lombok.*;" не пройдет проверку линтера, необходимо использовать построчный импорт.

Тест проверяет количество записей в бд - это условие не гарантирует того, что аккаунт был создан. Возможна ситуация, когда в бд уже есть запись, например админа, и тогда тест пройдет или наоборот - вбд не пустая. Лучше проверить был ли создан аккаунт в бд, например по email - обязательному параметру.

@uchitsa
Copy link
Contributor Author

uchitsa commented Aug 16, 2024

@SibirBear спасибо! Обновлено

@fey fey requested a review from Malcom1986 August 16, 2024 16:26
@fey
Copy link
Collaborator

fey commented Aug 16, 2024

@uchitsa hi, deploy please demo to render

@fey fey linked an issue Aug 16, 2024 that may be closed by this pull request
Make first name, last name be optional (empty) #286
Closed
@SibirBear
Copy link
Contributor

Проблема в том, что при создании аккаунта используется модель Account у которой аналогичные ограничения, что и у SignupAccountModel. Нужно изменить аннотации у модели Account и поправить тест - в нем ошибка в последней строчке.
Рекомендую беред комитами запускать проверку линтера и тесты.

@uchitsa
Copy link
Contributor Author

uchitsa commented Aug 16, 2024

@fey what does it mean "deploy please demo to render"? please describe

@uchitsa
Copy link
Contributor Author

uchitsa commented Aug 18, 2024

@Malcom1986 привет, просьба проверить ПР

@fey
Copy link
Collaborator

fey commented Aug 19, 2024

@uchitsa задеплойте приложение на render.com, чтобы можно было посмотреть изменения в вебе, потыкать кнопки, проверить работу форм и приложения.

@uchitsa
Copy link
Contributor Author

uchitsa commented Aug 23, 2024

@fey просьба скинуть мануал, инфо, ссылку, как задеплоить проект на render.com корректно

@uchitsa
Copy link
Contributor Author

uchitsa commented Aug 23, 2024

@fey 1 попытка прошла с ошибкой
Caused by: org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'liquibase' defined in class path resource [org/springframework/boot/autoconfigure/liquibase/LiquibaseAutoConfiguration$LiquibaseConfiguration.class]: java.lang.RuntimeException: Driver org.postgresql.Driver claims to not accept jdbcUrl, ${JDBC_DATABASE_URL}

@fey
Copy link
Collaborator

fey commented Aug 23, 2024

@uchitsa у меня нет под рукой рабочего мануала. по идее тут обычное spring приложение.
можете написать ребятам, которые делоили, например тут #284 @niyatanya

@SibirBear
Copy link
Contributor

@uchitsa в настройках приложения на Render нужно указать переменные для доступа к базе данных, для этого нужно создать базу данных Postgres на Render и перенести необходимые значения в настройки своего приложения. Приложение подключить из своего репозитоия на github.
Переменные должны быть:
JDBC_DATABASE_URL
JDBC_DATABASE_USERNAME
JDBC_DATABASE_PASSWORD

Вот статья в помощь https://ru.hexlet.io/blog/posts/render-java

@uchitsa
Copy link
Contributor Author

uchitsa commented Aug 26, 2024

@SibirBear полезный навык +1, спасибо

@uchitsa
Copy link
Contributor Author

uchitsa commented Aug 26, 2024

https://hexlet-correction-dl6y.onrender.com it works!
Please check PR @fey @Malcom1986 @SibirBear

@uchitsa uchitsa marked this pull request as draft August 26, 2024 16:40
@uchitsa uchitsa marked this pull request as ready for review August 26, 2024 16:40
Malcom1986
Malcom1986 reviewed Aug 27, 2024
View reviewed changes
src/test/java/io/hexlet/typoreporter/web/SignupControllerIT.java Outdated
@@ -53,6 +53,7 @@ static void datasourceProperties(DynamicPropertyRegistry registry) {

private static final String EMAIL_UPPER_CASE = "[email protected]";
private static final String EMAIL_LOWER_CASE = EMAIL_UPPER_CASE.toLowerCase();
private static final String EMPTY_NAME = "";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Давайте не будем это в статические свойства класса выносить. У нас ведь только один тест использует это свойство. Достаточно локальной переменной

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

исправлено

src/test/java/io/hexlet/typoreporter/web/SignupControllerIT.java Outdated
@@ -93,6 +94,23 @@ void createAccountWithIgnoreEmailCase() throws Exception {
assertThat(accountRepository.count()).isEqualTo(1L);
}

@Test
void createAccountWithEmptyNames() throws Exception {
String emptyNamesUser = "testEmptyNamesUser";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Лучше просто username, email, если считаете, что переменные нужны. А то сейчас диссонанс, называется empty, а по факту там не путсая строка

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

исправлено

@Malcom1986
Copy link
Collaborator

Ага, все работает. Поправьте еще пожалуйста пару земечаний по коду и будем мержить

@uchitsa
Copy link
Contributor Author

uchitsa commented Aug 27, 2024

@Malcom1986 обновлено с учётом замечаний

@Malcom1986 Malcom1986 merged commit 0c42ce9 into Hexlet:main Aug 28, 2024
1 check passed
@fey
Copy link
Collaborator

fey commented Aug 28, 2024

@uchitsa давайте еще форму обновим, удалим поля
image

@uchitsa uchitsa mentioned this pull request Aug 28, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Malcom1986 Malcom1986 Malcom1986 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Make first name, last name be optional (empty)
4 participants
@uchitsa @SibirBear @fey @Malcom1986