Hide "year in the books" for newly registered users

[rsk2]

Fixes #3187

[hughrun]

This is great! Just a couple of little things to fix and a suggestion to make it a little more specific.

[bSolt]

I'd like to help you out and add a test for this. Would that be alright with you? I'll probably try to create a PR that merges into your branch. :)

[bSolt]

Since we are filtering on the date as well, can we name this property "has_summary_read_throughs" instead?

[hughrun]

That does actually make sense and will avoid potential confusion in future.

I've also just realised the value could simply be len(readthroughs) because 0 is falsy.

@rsk2 do you mind making those two small changes? Then we'll stop bike-shedding this and I'll pull it in.

[rsk2]

OK, done!

[rsk2]

I'd like to help you out and add a test for this. Would that be alright with you? I'll probably try to create a PR that merges into your branch. :)

@bSolt Thanks :)

[bSolt]

Need to get better familiarized with the tests and patterns used here myself, actually. I am also new to the project and I had some trouble with how to test this specific feature. Getting this merged to fix the bug sooner is probably a better priority IMO.

[bSolt]

Let me know if you would like a primer on the tests anyway, but this is looking fine to me for now. Thanks!

[hughrun]

Need to get better familiarized with the tests and patterns used here myself, actually. I am also new to the project and I had some trouble with how to test this specific feature. Getting this merged to fix the bug sooner is probably a better priority IMO.

I'm planning to add some info to the docs about writing and updating tests, as it can be very confusing for people new to the project and/or Django (as I know from my own experience!) There have been some fairly urgent other things to resolve recently but I'll try to get back to that soon.

[hughrun]

Hmm, I think I have led you astray here with get_annual_summary_year. I'm looking into it.

[hughrun]

Ok @rsk2 sorry about this, I gave you some bad advice.

get_annual_summary_year returns this year if the current date is after 15 Dec and before 1 January, or last year if the current date is after 31 Dec but before 16 January. But if it's any other date, it returns None. I momentarily forgot this is the point of this utility function. It was all working because we were looking at this within that period of time. But now time has passed!

So that's why the test is failing: because you can't make a date with a year value of None.

However, you can fix this reasonably easily because we only need to know whether the user has_summary_readthroughs when get_annual_summary_year is True (otherwise the annual summary isn't displayed anyway):

cutoff = (
    date(get_annual_summary_year(), 12, 31)
    if get_annual_summary_year()
    else None
)
readthroughs = (
    models.ReadThrough.objects.filter(
        user=request.user, finish_date__lte=cutoff
    )
    if get_annual_summary_year()
    else []
)

[rsk2]

@hughrun no worries. Thanks for the fix.