Skip to content

Bugfix 641: Weekday is ignored in scheduled exports#680

Merged
codinguser merged 2 commits intocodinguser:developfrom
rivaldi8:bugfix-641-exports-weekday-ignored
Apr 27, 2017
Merged

Bugfix 641: Weekday is ignored in scheduled exports#680
codinguser merged 2 commits intocodinguser:developfrom
rivaldi8:bugfix-641-exports-weekday-ignored

Conversation

@rivaldi8
Copy link
Copy Markdown
Collaborator

Am I late for 2.2?

Please, check I haven't missed any corner case :)

The actions were run just once per week without taking into account the
weekdays set by the user.

Fixes codinguser#641
@codinguser
Copy link
Copy Markdown
Owner

codinguser commented Apr 25, 2017 via email

Copy link
Copy Markdown
Owner

@codinguser codinguser left a comment

Choose a reason for hiding this comment

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

Overall looks good to me, thanks.
Could you please check the failing tests on Travis though?
Do the tests need updating, or does the code break something?

@rivaldi8
Copy link
Copy Markdown
Collaborator Author

Sorry, I forgot to switch back to the full unit test suite before sending the pull request. It seems the tests have to be updated to set byDays for weekly actions.

Today I have a busy day. I'll fix it tomorrow.

@rivaldi8
Copy link
Copy Markdown
Collaborator Author

It should be fixed now.

@codinguser
Copy link
Copy Markdown
Owner

Cool, thanks. So byday is always set by the recurrence dialog, right?

@codinguser codinguser merged commit 893132a into codinguser:develop Apr 27, 2017
@rivaldi8
Copy link
Copy Markdown
Collaborator Author

Yes, the dialog forces you to set at least on weekday. However, after a more thorough inspection, I've found GncXmlHandler never sets the recurrence's byDays. It doesn't parse the weekdays.

Until we don't implement the parsing for imported weekly transaction, I'd add at least one weekday to byDays. We could just use the weekday of the start date. That way it would be executed at least once per week. In fact I'd say that's how it worked until now.

Also, although with a warning, GnuCash desktop allows you to define a weekly transaction without setting any weekday. In that case, it's never executed. So, I'll have to add a check for empty byDays too.

@codinguser
Copy link
Copy Markdown
Owner

@rivaldi8 sounds like a plan 👍

@rivaldi8 rivaldi8 deleted the bugfix-641-exports-weekday-ignored branch April 29, 2017 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants