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

No error if account has no bookings #5

Merged
merged 1 commit into from
Nov 25, 2015
Merged

No error if account has no bookings #5

merged 1 commit into from
Nov 25, 2015

Conversation

luxflux
Copy link
Collaborator

@luxflux luxflux commented Nov 24, 2015

This prevents NoMethodError in Accounts#show: undefined method 'value_date' for nil:NilClass.

@stevschmid
Copy link

👍

@huerlisi
Copy link
Owner

Wow, that's old code:-) Remember the days when we had to use h to escape everything;-)

I'll merge this as it fixes the issue. This part of code needs some love though to get it up to date and fix the probably related issue in Bookyt: huerlisi/bookyt#71

I'll do a release today or tomorrow. Thanks for the fix!

huerlisi added a commit that referenced this pull request Nov 25, 2015
@huerlisi huerlisi merged commit 1c179c0 into huerlisi:master Nov 25, 2015
@luxflux luxflux deleted the fix-account-without-bookings-display branch November 25, 2015 13:48
@huerlisi
Copy link
Owner

I'm working on some more cleanup in this area. Especially to also fix huerlisi/bookyt#71

I try to understand how this issue could show up in the first place. It looks like the only place where we use these partials is in https://github.com/huerlisi/has_accounts_engine/blob/master/app/views/accounts/_show_bookings.html.haml#L17 where we already check for an empty @bookings var.

Could you point me to the instances the issue showed up, so I can ensure I won't re-introduce it.

@luxflux
Copy link
Collaborator Author

luxflux commented Dec 1, 2015

Well, we had the error after we created a new tenant and loaded the de-CH seeds in Bookyt.
There is a default account plan created where all of the accounts raised this error.

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.

None yet

3 participants