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

[Feature] Adding "Leave Page" Functionality To Invoices, Credits and Purchase Orders #2317

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

Conversation

Civolilah
Copy link
Collaborator

@beganovich @turbo124 The PR includes adding "leave page" functionality to invoices, credits, and purchase orders. Let me know your thoughts.

@Civolilah
Copy link
Collaborator Author

@turbo124 I looked into the logic again and while I was working on implementing this functionality for Purchase Orders, I noticed that we can exclude one more property from the comparison between objects (similar to what we did for public_notes, private_notes, terms, and footer). I noticed that we are generating _id for line items every time we load the page. I think it is completely unnecessary to compare this property with the initial object because it always has a new value and doesn't affect the functionality. So, to make the functionality more secure, I excluded just the _id prop from line items from the comparison. I hope you agree with me also.

@turbo124
Copy link
Member

@Civolilah

need to ensure these new nav buttons are included in this functionality:

image

@Civolilah
Copy link
Collaborator Author

@Civolilah

need to ensure these new nav buttons are included in this functionality:

image

@turbo124 Sure, I've added those button functionalities. One additional thing was done: while testing the functionality, I noticed that the add_comment action is a similar "type" of action as mark_sent, mark_paid etc., which we excluded from the functionality. So to maintain great UX, I've excluded the functionality only from this action. Let me know your thoughts.

@turbo124
Copy link
Member

@Civolilah just to confirm, which input fields are excluded from monitoring?

@Civolilah
Copy link
Collaborator Author

@Civolilah just to confirm, which input fields are excluded from monitoring?

@turbo124 So, we've excluded public_notes, private_notes, terms, and footer (basically all markdown editor fields) and I've excluded the id prop from line_items because we always generate a new one. It doesn't make sense to track it if a new one will always be created.

Comment on lines +97 to +98
!path?.includes('.') ||
(path?.includes('line_items') && path?.split('.')?.[2] === '_id')
Copy link
Member

Choose a reason for hiding this comment

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

Ugh, please explain change on this and logic behind it, seems weird.

Also, please add it as comment for future reference.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@beganovich Sure, the comment has been added. This logic aims to exclude unnecessary properties from tracking.

The _id property for each line item is newly generated whenever we load the edit page, eliminating the need to track them at all. Paths are generated conventionally - properties on the first level of an object use just the property name, while nested properties use dots between levels. For example, a path like 'line_items.0._id' indicates that if the path includes 'line_items' and the property is '_id', we exclude that path from tracking. This logic will only exclude _id properties from line items since it specifically targets these paths. All other line item properties will be tracked normally.

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

Successfully merging this pull request may close these issues.

3 participants