Use specific subject line in update emails

Converted latest patch to an MR and added tests.

The test no longer uses exact counts for the calls to the mocked functions because they vary so much now. If exact counts are wanted they would have to be added to the data provider array. That doesn't seem necessary to me for this test.

The test no longer uses exact counts for the calls to the mocked functions because they vary so much now.

Should a follow up be opened to add back in a different way for performance measuring? Seems like coverage we could be losing.

Ran the tests locally since I can't trigger test-only on the MR

Failed asserting that two strings are identical.
Expected :'Site notice for Test site'
Actual :'New release(s) available for Test site'

Change there seems fine so going to mark, but still wonder about a follow up.,

Hrm, "Site notice for @site_name" seems pretty unhelpful and very easy to ignore.

I'm in favor of adding "security" to the subject in the case of missing security updates, but the other change here seems like a step backwards, not forwards.

The original report was a complaint that it shouldn't say "New releases(s) available for @site_name" if there was a problem checking for updates. That could be fixed by checking the reason code (like we're now doing to detect missing security updates) and change the subject if it's an error to fetch.

Also, if this is really a bug, the title of this issue probably shouldn't be "improve update status email subject". 😅 If that's the scope of the issue, it should be a task, not a bug. And the new subject should actually be an improvement. 😂

IMHO, either we should focus on fixing the originally reported bug (that the subject shouldn't say "New release(s) available..." if there's a problem checking for updates), or we should change this to a task and make real improvements.

Sorry,
-Derek

@dww, thanks for the review.

I am leaving this as a bug since it is odd to get a message with a subject that releases are available when the body says that there was trouble getting the release information. I have updated the proposed resolution with my idea for handing this.

Couldn't run test-only feature but ran locally shouldn't "contrib, fetch fail" and "core fetch fail" fail also?

In this case they should not fail. Prior to this change there were no tests for fetching failures. These test are showing that the existing behavior does not change when there are both updates and fetch failures. That is, the email subject will always announce that new releases are available even if there are fetch failures for another project.

Thanks for following up #quietone. In that case believe this is good.

Thanks @smustgrave for the manual testing and question; @quietone's response helped me understand the logic better.

I've posted just a few small improvements to grammar on the MR, plus typehinted the rest of the test method. It's on the wobbly line for scope but we're allowed to break test hint signatures at any point, and it helped me understand the data provider structure for reviewing all the new cases. (Plus it will save work down the road.)

Once those are fixed up I think this is ready.

Saving issue credits.

pradhumanjain2311 → made their first commit to this issue’s fork.

Thanks @pradhumanjain2311 for your contributions. The point of suggestions is for the existing contributors to the issue to have a chance to +1 the reviewer's suggested changes. A person who has not otherwise contributed to the issue just clicking buttons to apply the suggestions does not really help move the issue forward. Therefore, I won't be crediting this contribution. In the future, you can receive credit by providing a review of an issue with open questions.

Marking NR for one of the issue's contributors to review the applied suggestions.

Does appear as suggestions were applied.

@smustgrave, again, I don't need validation that the suggestions were applied -- I can see that from the tooling -- I need validation that they were good suggestions. Thanks! :)

Left comments on the threads

3 agree, 1 I'm 50/50, 1 I made a suggestion.

The 1 suggestion can be done probably directly from gitlab but didn't move to NW @xjm agree on it?

Moving to NW for the 1 suggestion, which seems like a good task. Also could use a rebase being 300+ commits behind.