Report the correct number of pushes on the feeds

[zeripath]

Unfortunately #16282 contained a regression whereby it was assumed that the []*Commits in PushCommits retained its full length and therefore the Len field was dropped.

This has led to the incorrect reporting of the number of commits in pushes which are larger than setting.UI.FeedMaxCommitNum.

Prior to #16282 the Len was reported on the push commits so mostly we can just drop back to using this, and restart adding it back in.

However, for pushes after this - AFAICS this information is lost and is not recalcuable. Therefore these pushes will be limited to FeedMaxCommitNum.

Fix #16804

Signed-off-by: Andrew Thornton art27@cantab.net

[codecov-commenter]

Codecov Report

Merging #16811 (7129981) into main (28ac4a7) will increase coverage by 0.00%.
The diff coverage is 77.77%.

@@           Coverage Diff           @@
##             main   #16811   +/-   ##
=======================================
  Coverage   45.46%   45.47%           
=======================================
  Files         762      762           
  Lines       86019    86027    +8     
=======================================
+ Hits        39111    39122   +11     
+ Misses      40593    40591    -2     
+ Partials     6315     6314    -1     

Impacted Files	Coverage Δ
modules/templates/helper.go	51.36% <0.00%> (-0.18%)	⬇️
modules/repository/commits.go	82.85% <100.00%> (+1.03%)	⬆️
services/pull/pull.go	41.78% <0.00%> (-0.41%)	⬇️
modules/queue/unique_queue_disk_channel.go	48.63% <0.00%> (+1.36%)	⬆️
modules/queue/workerpool.go	55.72% <0.00%> (+1.90%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 28ac4a7...7129981. Read the comment docs.

[zeripath]

I could write a migration to "backfill" and attempt to recalculate Len but I'm not certain if it's the right thing - as it won't necessarily be right. It would also be very slow.

I suppose we could actually make the Len field a string, so that for any actions that represent commits/pushes but don't contain a Len whilst do have len(Commits) >= setting.UI.FeedMaxCommitNum we set the Len to "≥" + setting.UI.FeedMaxCommitNum but I'm not certain it's entirely worth it - this could also be done at the template level.

[KN4CK3R]

I don't think we should restrict the commit list in pushUpdates.

actionNotifier.NotifyPushCommits uses the commits and it may be reasonable to limit the list
indexerNotifier.NotifyPushCommits does not use the commits and it may be an error if the list was stripped
webhookNotifier.NotifyPushCommits uses the commits and it may be reasonable to limit the list

Now the question is if both limits should be identical. Personally I think webhooks should get more data than the ui notification.
For webhooks Github [1] and Gitlab [2] deliver 20 commits. Gitlab adds a total_commits_count attribute with the real count.
So it may be good to limit the list inside NotifyPushCommits only and provide a TotalCommitsCount for the receivers.

[1] https://docs.github.com/en/developers/webhooks-and-events/webhooks/webhook-events-and-payloads#push
[2] https://docs.gitlab.com/ee/user/project/integrations/webhooks.html#push-events

[zeripath]

We still need to use and create Len because we're not going to be able to rely on this not being truncated.

[techknowlogick]

backported here: #16822