Skip to content
This repository has been archived by the owner on Jan 26, 2021. It is now read-only.

[#139] Add a validation to enable/disable the save button of the Edit Profile view #143

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

ednofedulo
Copy link

@ednofedulo ednofedulo commented Oct 4, 2020

Description

Add a validation to enable/disable the save button of the Edit Profile view based on the state of the name field.

Fixes #139

Type of Change:

  • Code
  • User Interface

Code/Quality Assurance Only

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

In the Edit Profile view, empty the name field and the save button should be disabled.

Enabled Disabled

Checklist:

  • My PR follows the style guidelines of this project
  • I have performed a self-review of my own code or materials

Code/Quality Assurance Only

  • My changes generate no new warnings
  • New and existing unit tests pass locally with my changes

@ednofedulo ednofedulo changed the title [#139] [#139] Add a validation to enable/disable the save button of the Edit Profile view Oct 4, 2020
@@ -79,7 +86,7 @@ struct ProfileEditor: View {
self.profileViewModel.inActivity = true
// make network call to update profile
self.updateProfile()
})
}.disabled(self.canSave == false))
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can do something like .disabled(editProfileData.name.isEmpty)

Comment on lines 15 to 21
var canSave:Bool {
guard let name = $editProfileData.name.wrappedValue, name.isEmpty == false else {
return false
}
return true
}

Copy link
Contributor

Choose a reason for hiding this comment

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

We won't need this code then.

Copy link
Author

Choose a reason for hiding this comment

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

i made this way thinking about future proof, if later need to add another required field or another validation to enable/disable the button.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay. In that case, we can define this computed property like this: 'return editProfileData.name.isEmpty'
Second, please add a unit test for this new computed property.

Copy link
Author

Choose a reason for hiding this comment

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

done

- make canSave computed property more clean
Comment on lines 151 to 180
func testCanSaveProfileWithoutNameAndFail() throws {
// Profile Service
let profileService: ProfileService = ProfileAPI(urlSession: urlSession)

// View Model
let sampleData = ProfileModel.ProfileData(id: 0, name: "", username: "", email: "")
let profileVM = ProfileViewModel()
profileVM.saveProfile(profile: sampleData)

// Profile Editor View
let profileEditor = ProfileEditor(profileService: profileService, profileViewModel: profileVM)

XCTAssertFalse(profileEditor.canSave)
}

func testCanSaveProfileWithNameAndSucceed() throws {
// Profile Service
let profileService: ProfileService = ProfileAPI(urlSession: urlSession)

// View Model
let sampleData = ProfileModel.ProfileData(id: 0, name: "name", username: "", email: "")
let profileVM = ProfileViewModel()
profileVM.saveProfile(profile: sampleData)

// Profile Editor View
let profileEditor = ProfileEditor(profileService: profileService, profileViewModel: profileVM)

XCTAssertTrue(profileEditor.canSave)
}

Copy link
Contributor

Choose a reason for hiding this comment

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

We probably don't need a throwing function. Also, can we improve the naming of these test functions?

Copy link
Author

Choose a reason for hiding this comment

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

ok. i changed the name based on another test function that already exists

…e save button on the edit profile view and merge into a single method the scenarios
@sunjunkie
Copy link
Contributor

Please upload smaller files for the screenshots.

@@ -12,6 +12,10 @@ struct ProfileEditor: View {
@State var editProfileData = ProfileViewModel().getEditProfileData()
@ObservedObject var profileViewModel = ProfileViewModel()

var canSave:Bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

Change name to enableSaveButton/disableSaveButton. More descriptive.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add validation for empty name in edit profile.
4 participants