Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Journal Entry Export #52
Journal Entry Export #52
Changes from 14 commits
e066cb0
0652db4
f40b602
44c8043
a0f1ec7
0686373
d637fe5
0347390
ac14610
937e374
eb8d4ff
52cab90
f3b5841
eb9343f
dbc13b0
95022ca
af0da4f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
get_expense_comment
method constructs a comment for an expense based on theexpense_memo_structure
fromAdvancedSetting
. It dynamically builds the comment by iterating over the memo structure and appending details from the expense. The method correctly handles the case where certain details may not be present. However, there is a bug in the loop that concatenates the fields; it appends a hyphen after every field, which will result in an incorrect comment format.Committable suggestion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The extended search output confirms that the
get_invoice_date
method inbase_model.py
returns the current date in the format '%Y-%m-%d' when none of the expected keys are present or their values are empty. This format does not match the format specified in the method's docstring ('%Y-%m-%dT%H:%M:%S'). Therefore, there is an inconsistency between the docstring and the actual behavior of the method.The review comment is correct in pointing out that the method should ensure that the date format returned is consistent with the expected format mentioned in the docstring. The current implementation does not do this when it falls back to the current date.
Analysis chain
The
get_invoice_date
method extracts the invoice date from various fields within thedescription
of anAccountingExport
object. It falls back to the current date if none of the expected keys are present. This method should ensure that the date format returned is consistent and matches the expected format '%Y-%m-%dT%H:%M:%S' as mentioned in the docstring.Scripts Executed
The following scripts were executed for the analysis:
Script:
Length of output: 0
Script:
Length of output: 1421
Script:
Length of output: 1926
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The methods
get_vendor_id
andget_employee_id
are placeholders returning empty strings. If these methods are meant to be implemented, add a TODO comment or implement them. If they are not needed, consider removing them to avoid confusion.