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

Feat/add on click to update applied on date #81

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,10 @@ FORMAT: lastname, firstname
- [Github](https://github.com/stefanjbloom)
- [LinkedIn](https://www.linkedin.com/in/stefanjbloom/)

**Cardona, Danielle**
- [Github](https://github.com/dcardona23)
- [LinkedIn](https://github.com/dcardona23)
Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like you accidentally listed your Github twice instead of adding the LinkedIn link.


**Chirchirillo, Joe**
- [Github](https://github.com/jchirch)
- [LinkedIn](https://www.linkedin.com/in/joechirchirillo/)
Expand Down
9 changes: 9 additions & 0 deletions cypress/e2e/jobsApplicationSpec.cy.js
Copy link
Collaborator

Choose a reason for hiding this comment

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

You could add a cypress test to verify that the data is saving in the back end (see the handleSave() function notes for additional context.

Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,15 @@ describe("View specific job app page with all fields filled in", () => {
});
});

it("edits an application date", () => {
cy.wait("@showSingleJobApp");
cy.get("p.font-medium").
within(() => {
cy.get("span.font-semibold").click()
})
cy.get("#dateApplied").type('2025-01-01').should('have.value', '2025-01-01')
})

it("displays notes and edit button", () => {

cy.wait("@showSingleJobApp");
Expand Down
32 changes: 28 additions & 4 deletions src/components/pages/showJobApplication.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ function JobApplication() {
const [jobDescription, setJobDescription] = useState('');
const [applicationURL, setApplicationURL] = useState('');
const [companyId, setCompanyId] = useState('');
const [isEditing, setIsEditing] = useState(false);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could this be editing and setEditing instead of isEditing and setIsEditing?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree with you that editing and setEditing read better, but I was trying to maintain consistency with other boolean state syntax in showJobApplication.tsx. For example, modal state is managed through "isModalOpen, setIsModalOpen", and edit model state is managed through "isEditModelOpen, setIsEditModelOpen."

Copy link
Collaborator

Choose a reason for hiding this comment

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

I like the consistency that Danielle applied. It might be good to add an issue/ticket to refactor all of the "is" parts in the code into simply "set".

const [editedDate, setEditedDate] = useState(jobApp ? jobApp.date_applied : '')

useEffect(() => {
if (jobAppId) {
Expand All @@ -62,6 +64,7 @@ function JobApplication() {
setJobDescription(data.data.attributes.job_description)
setApplicationURL(data.data.attributes.application_url)
setCompanyId(data.data.attributes.company_id)
setEditedDate(data.data.attributes.date_applied ? data.data.attributes.date_applied.toString() : '')

} catch (err) {
console.error("Failed to fetch job application:", err);
Expand All @@ -72,6 +75,11 @@ function JobApplication() {
}
}, [jobAppId]);

const handleSave = () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't believe that the handleSave() function is updating the back end. If you call updateJobApplication() inside of the handleSave function, it would trigger the updateJobApplication() function so that the changes persist in the backend database.

You could add a cypress test to verify that the data is saving in the back end.

setIsEditing(false)
setEditedDate(editedDate)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Because editedDate is already the current value in state, this code on line 80, setEditedDate(editedDate) may not be doing anything. It can likely be removed.
*Note: editedDate is updated with the user types a new date into the input field and set via the onChange event setEditedDate(e.target.value)

}

const openModal = () => setIsModalOpen(true);
const closeModal = () => setIsModalOpen(false);

Expand All @@ -91,7 +99,7 @@ function JobApplication() {
job_description: jobDescription,
application_url: applicationURL
}
console.log(compileData)

updateJobApplication(compileData)
.then((updatedApplication) => {
console.log("Application updated successfully:", updatedApplication);
Expand Down Expand Up @@ -122,9 +130,25 @@ function JobApplication() {
</Link>
<p className="font-medium mb-4">
Applied On:{" "}
<span className="font-semibold">
{`${jobApp.date_applied}`}
</span>
{isEditing ? (
<input
type="date"
id="dateApplied"
value={editedDate instanceof Date ? editedDate.toString().split('T')[0] : editedDate}
onChange={(e) => setEditedDate(e.target.value)}
className="p-2 border-4 border-slate-800 rounded-lg focus:outline-none focus:ring-2 transition-all duration-200 ease-in-out"
onBlur={handleSave}
required
/>
) : (
<span
className="font-semibold cursor-pointer underline underline-dashed hover:text-blue-500 transition-all duration-150 ease-in-out"
onClick={() => setIsEditing(true)}
>
{editedDate instanceof Date ? editedDate.toString().split('T')[0] : editedDate}
</span>
)}

</p>
<p className="mb-6">
Status:{" "}
Expand Down