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

AYS-661 | Refactor email validation limits #431

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ public class AdminRegistrationApplicationCompleteRequest {

@EmailAddress
@NotBlank
@Size(min = 2, max = 255)
private String emailAddress;

@NotBlank
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ public class AysPasswordForgotRequest {

@EmailAddress
@NotBlank
@Size(min = 2, max = 255)
Copy link
Contributor

Choose a reason for hiding this comment

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

Size kontrolu buradan kaldirilmis ancak @EmailAddress anotasyonu icerisine eklenmemis, o eklemeyi yapabilir miyiz?

Copy link
Author

Choose a reason for hiding this comment

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

Size kontrolünü'de @agitrubard isteği üzerine kaldırdım. Kontrol saten EmailAddress içinde yapılıyormuş. Ekstra bir şey eklememe gerek var mı yine de?

Copy link
Contributor

Choose a reason for hiding this comment

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

Onceki commitinde hem burada @Size anotasyonu eklenmis hem de @EmailAddress anotasyonunun icerisine size kontrolu eklenmisti. Dolayisiyla @Size anotasyonunu kaldirmak yeterli olacakti ancak sonraki committe ikisi birden kaldirilmis. @EmailAddress anotasyonunun icerisine tekrar ekleme yapabilir misin?

Copy link
Author

Choose a reason for hiding this comment

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

Tamamdır gözümden kaçmış affola.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hic onemli degil, yardimci olabildiysem ne mutlu bana 🙌

Copy link
Author

Choose a reason for hiding this comment

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

@emailaddress interface sınıfına hiç dokunmamıştım. Eğer commit mesajımla alakalı bahsetmişseniz ben orada emailAddress attribute'unun üstündeki anatasyon olarak belirtmek istedim. Eğer yanlışım varsa düzeltin çünkü commitlerimin hepsine baktım ama hiç ellememişim interface sınıfını.

Copy link
Contributor

Choose a reason for hiding this comment

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

EmailAddressValidator sınıfındaki bu değişiklikleri kastetmiştim
IMG-20250117-WA0016.jpg

Copy link
Author

Choose a reason for hiding this comment

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

Kusuruma bakmayın gözümden kaçmış şimdi düzelttim commit ettim.

Copy link
Contributor

Choose a reason for hiding this comment

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

Rica ederim eline saglik 🙌🏼

private String emailAddress;

}
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ public class AysUserCreateRequest {

@EmailAddress
@NotBlank
@Size(min = 2, max = 255)
private String emailAddress;

@Valid
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ public class AysUserUpdateRequest {

@EmailAddress
@NotBlank
@Size(min = 2, max = 255)
private String emailAddress;

@Valid
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class EmailAddressValidator implements ConstraintValidator<EmailAddress, String>
* <li>u@[email protected]</li>
* <li>[email protected]</li>
* <li>user</li>
* <li>[email protected]</li>
* </ul>
*
* @param email object to validate
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@

/**
* Represents an emergency evacuation application.
* Extends {@link BaseDomainModel} and includes details such as personal and contact information,
* Extends {@link BaseDomainModel} and includes details such as personal and
* contact information,
Copy link
Contributor

Choose a reason for hiding this comment

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

Burayi eski haline cevirebiliriz

* location details, and application status.
*/
@Getter
Expand Down Expand Up @@ -42,7 +43,6 @@ public class EmergencyEvacuationApplication extends BaseDomainModel {

Copy link
Collaborator

Choose a reason for hiding this comment

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

@egehanasal'ın yazdıklarına ek olarak burayı da eski hâline alabiliriz

Copy link
Author

Choose a reason for hiding this comment

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

Screenshot 2025-01-18 192604

Hocam boşluktan bahsediyorsanız bende boşluk gözükmüyor. Ben formatı ellemediğim halde neden benim commitlerim formatı bozuyor?

private Institution institution;


Copy link
Contributor

Choose a reason for hiding this comment

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

Burayi eski haline cevirebiliriz

/**
* Checks if the application does not have an associated institution.
*
Expand All @@ -62,7 +62,6 @@ public boolean isInstitutionOwner(final String institutionId) {
return this.institution.getId().equals(institutionId);
}


Copy link
Contributor

Choose a reason for hiding this comment

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

Burayi eski haline cevirebiliriz

/**
* Marks the emergency evacuation application as pending.
* Generates a reference number and updates the status to pending.
Expand All @@ -72,12 +71,14 @@ public void pending() {
this.referenceNumber = AysRandomUtil.generateNumber(10).toString();
this.status = EmergencyEvacuationApplicationStatus.PENDING;
this.isInPerson = this.applicantPhoneNumber == null;
this.hasObstaclePersonExist = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

Bu degisiklige nicin ihtiyacimiz oldu 🤔

Copy link
Author

Choose a reason for hiding this comment

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

yaptığım değişikliklerden sonra test'lerden geçemedi. Null set edilmediği için hata almıştım. Bundan kaynaklı null değeri atadım oraya ve testlerden sorunsuz geçti.

Copy link
Contributor

Choose a reason for hiding this comment

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

Bu alanla alakali degisiklikleri bu issue ozelinde yapmamiza gerek olmadigi icin, degisiklikleri ve testlerdeki eklemeleri geri alabiliriz

}

/**
* Sets the institution ID for the application.
*
* @param institutionId the ID of the institution to associate with the application.
* @param institutionId the ID of the institution to associate with the
* application.
Copy link
Contributor

Choose a reason for hiding this comment

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

Burayi eski haline cevirebiliriz

*/
public void setInstitutionId(final String institutionId) {
this.institution = Institution.builder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@

/**
* A JPA entity class that represents an emergency evacuation entity.
* The emergency evacuation applications are defined in the AYS_EMERGENCY_EVACUATION_APPLICATION table in the database.
* The emergency evacuation applications are defined in the
* AYS_EMERGENCY_EVACUATION_APPLICATION table in the database.
Copy link
Contributor

Choose a reason for hiding this comment

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

Burayi eski haline cevirebiliriz

Copy link
Author

Choose a reason for hiding this comment

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

@agitrubard son requested changes'larına göre düzenledim. Tekrar bu haline mi çevireyim?

Copy link
Contributor

Choose a reason for hiding this comment

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

Bu javadoc'larin degistirilmesine gerek yok, maindeki haline cevirebiliriz eger @agitrubard icin de uygunsa

Copy link
Author

Choose a reason for hiding this comment

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

Bunları kullandığım formatter dan kaynaklı bozulmuştu. @agitrubard saten bana geri çevirmem için uyardı ama ben bunları gözden kaçırmışım hemen hallediyorum.

*/
@Entity
@Getter
Expand Down Expand Up @@ -98,7 +99,6 @@ public class EmergencyEvacuationApplicationEntity extends BaseEntity {
@Column(name = "NOTES")
private String notes;


Copy link
Contributor

Choose a reason for hiding this comment

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

Burayi eski haline cevirebiliriz

@OneToOne
@JoinColumn(name = "INSTITUTION_ID", insertable = false, updatable = false)
private InstitutionEntity institution;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,11 @@
import org.ays.common.util.validation.Name;
import org.hibernate.validator.constraints.Range;


Copy link
Contributor

Choose a reason for hiding this comment

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

Burayi eski haline cevirebiliriz

/**
* Represents a request to complete emergency evacuation request. The request includes fields for the required user
* information, such as the user's phone number, as well as their first and last name.
* Represents a request to complete emergency evacuation request. The request
* includes fields for the required user
* information, such as the user's phone number, as well as their first and last
* name.
agitrubard marked this conversation as resolved.
Show resolved Hide resolved
*/
@Getter
@Setter
Expand Down Expand Up @@ -64,7 +65,6 @@ public class EmergencyEvacuationApplicationRequest {
@Size(min = 2, max = 100)
private String targetDistrict;


Copy link
Contributor

Choose a reason for hiding this comment

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

Burayi eski haline cevirebiliriz

@Name
@Size(min = 2, max = 100)
private String applicantFirstName;
Expand All @@ -76,22 +76,24 @@ public class EmergencyEvacuationApplicationRequest {
@Valid
private AysPhoneNumberRequest applicantPhoneNumber;

private Boolean hasObstaclePersonExist;
Copy link
Contributor

Choose a reason for hiding this comment

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

Bu degisiklige nicin ihtiyacimiz oldu 🤔

Copy link
Author

Choose a reason for hiding this comment

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

testleri build ederken hasObstaclePersonExist olmadığına dair ve bundan kaynaklı hata almıştım. Attribute olarak eklediğimde testlerden geçti.

Copy link
Contributor

Choose a reason for hiding this comment

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

Testlerin tamami mainde geciyor. Yaptigin baska degisiklikler sonucu bu hatalari almis olmalisin. Bu alan 661 issue'sunun scope'unda olmadigi icin bunlari kaldirip testleri de ona gore duzeltebilir misin?

Copy link
Author

Choose a reason for hiding this comment

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

Tamam dır. Şeyi sormak isterim ben 609 issue'sunu yaparken bu değişiklikleri yapmıştım. Bu değişiklikleri 609 branch'ine alıp tekrar commit atayım mı? Yoksa 609 issue'suna dokunmayayım mı?

Copy link
Contributor

Choose a reason for hiding this comment

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

Bu degisiklikler baska bir issue'ya aitse onun branch'inde olmasi gerekir, ancak 609 su an icin backlog'da. Ready'de olan issue'lari kendimize assign edip calismaya oyle basliyoruz. Eger ek sorularin olursa jira'da issue uzerinden konusalim beni etiketleyebilirsin

Copy link
Author

Choose a reason for hiding this comment

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

Teşekkürler.


@JsonIgnore
Copy link
Collaborator

Choose a reason for hiding this comment

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

@egehanasal 'ın söylediklerine ek olarak burayı da eski hâline alabiliriz. Üstünde bir boşluk vardı bu silinmiş

Suggested change
@JsonIgnore
@JsonIgnore

@AssertTrue(message = "all applicant fields must be filled")
@SuppressWarnings("This method is unused by the application directly but Spring is using it in the background.")
private boolean isAllApplicantFieldsFilled() {

if (StringUtils.isEmpty(this.applicantFirstName) && StringUtils.isEmpty(this.applicantLastName) && this.applicantPhoneNumber == null) {
if (StringUtils.isEmpty(this.applicantFirstName) && StringUtils.isEmpty(this.applicantLastName)
&& this.applicantPhoneNumber == null) {
agitrubard marked this conversation as resolved.
Show resolved Hide resolved
return true;
}


Copy link
Collaborator

Choose a reason for hiding this comment

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

@egehanasal 'ın söylediklerine ek olarak burayı da eski hâline alabiliriz.

return !StringUtils.isBlank(this.applicantFirstName) && !StringUtils.isBlank(this.applicantLastName)
&&
this.applicantPhoneNumber != null
&&
!(StringUtils.isBlank(this.applicantPhoneNumber.getCountryCode()) && StringUtils.isBlank(this.applicantPhoneNumber.getLineNumber()));
!(StringUtils.isBlank(this.applicantPhoneNumber.getCountryCode())
&& StringUtils.isBlank(this.applicantPhoneNumber.getLineNumber()));
agitrubard marked this conversation as resolved.
Show resolved Hide resolved
}

@JsonIgnore
Expand All @@ -103,7 +105,8 @@ private boolean isPhoneNumberMustNotBeSameOne() {
return true;
}

if (StringUtils.isEmpty(this.applicantPhoneNumber.getLineNumber()) || StringUtils.isEmpty(this.phoneNumber.getLineNumber())) {
if (StringUtils.isEmpty(this.applicantPhoneNumber.getLineNumber())
|| StringUtils.isEmpty(this.phoneNumber.getLineNumber())) {
agitrubard marked this conversation as resolved.
Show resolved Hide resolved
return true;
}

Expand All @@ -115,11 +118,12 @@ private boolean isPhoneNumberMustNotBeSameOne() {
@SuppressWarnings("This method is unused by the application directly but Spring is using it in the background.")
private boolean isSourceCityAndDistrictDifferentFromTargetCityAndDistrict() {

if (this.sourceCity == null || this.sourceDistrict == null || this.targetCity == null || this.targetDistrict == null) {
if (this.sourceCity == null || this.sourceDistrict == null || this.targetCity == null
|| this.targetDistrict == null) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if (this.sourceCity == null || this.sourceDistrict == null || this.targetCity == null
|| this.targetDistrict == null) {
if (this.sourceCity == null || this.sourceDistrict == null || this.targetCity == null || this.targetDistrict == null) {

Copy link
Collaborator

Choose a reason for hiding this comment

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

Bu format düzenlemesi henüz yapılmamış

return true;
}
}

if (!this.sourceCity.equalsIgnoreCase(this.targetCity)){
if (!this.sourceCity.equalsIgnoreCase(this.targetCity)) {
return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,9 @@ public EmergencyEvacuationRequestBuilder() {

public EmergencyEvacuationRequestBuilder withValidValues() {
return this
.withSeatingCount(1)
.withPhoneNumber(new AysPhoneNumberRequestBuilder().withValidValues().build())
.withApplicantPhoneNumber(new AysPhoneNumberRequestBuilder().withValidValues().build())
.withAddress("Lorem Ipsum is simply dummy text of the printing and typesetting industry.");
.withAddress("Lorem Ipsum is simply dummy text of the printing and typesetting industry.")
.withApplicantPhoneNumber(new AysPhoneNumberRequestBuilder().withValidValues().build());
Copy link
Contributor

Choose a reason for hiding this comment

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

Bu degisiklige nicin ihtiyacimiz oldu 🤔

Copy link
Author

Choose a reason for hiding this comment

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

Bir önceki requestimde testlerden hata aldığım için ben de diğer methodlar eksik diye mi hata alıyorum acaba diye method eklemiştim ama gerek yokmuş geri kaldırdım.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Burayı eski hâline alabilir miyiz?

}

public EmergencyEvacuationRequestBuilder withFirstName(String firstName) {
Expand Down
Loading