open-sauced / app

🍕 Insights into your entire open source ecosystem.
https://pizza.new
Apache License 2.0
426 stars 225 forks source link

Bug: Spam report isn't consistent on repo insights #2781

Closed BekahHW closed 8 months ago

BekahHW commented 8 months ago

Describe the bug

The cards for spam and the table row, should show the same number. However that's not happening.

Screenshot 2024-02-27 at 10 23 16 AM

Steps to reproduce

You can see it here https://app.opensauced.pizza/pages/BekahHW/1229/dashboard.

Browsers

No response

Additional context (Is this in dev or production?)

No response

Code of Conduct

Contributing Docs

github-actions[bot] commented 8 months ago

Thanks for the issue, our team will look into it as soon as possible! If you would like to work on this issue, please wait for us to decide if it's ready. The issue will be ready to work on once we remove the "needs triage" label.

To claim an issue that does not have the "needs triage" label, please leave a comment that says ".take". If you have any questions, please reach out to us on Discord or follow up on the issue itself.

For full info on how to contribute, please check out our contributors guide.

jpmcb commented 8 months ago

This probably has an inconsistency similar to what needed to be fixed in #2574 - looking into it now!

jpmcb commented 8 months ago

I think the problem is that the UX is confusing:

Screenshot 2024-02-27 at 10 36 11 AM

After digging into the code, the big number in the card is actually the stat for "today" and the little number is yesterdays stat:

https://github.com/open-sauced/app/blob/41109a20f3669c2c9d8f4db9128c3d289155efbc/components/organisms/Dashboard/dashboard.tsx#L23-L24

and since we are generating those stats based on the pull requests histogram (where each bucket is a day), we are missing the 2 spam pull requests since they were from a few weeks ago:

Screenshot 2024-02-27 at 10 34 35 AM

I see a few paths forward:

  1. We unify the stats from within the rows of repos and what appears in the card and update the cards to be a total of spam PRs. But maybe that isn't that useful: seeing a change over time from today --> yesterday could be more interesting than just seeing the totals of a range of time.
  2. We make the cards more clear what they are displaying since there's not alot to show what exactly they are displaying.

cc @isabensusan

BekahHW commented 8 months ago

@jpmcb I was under the impression that those cards should be showing numbers based on the selected filter (7 days, 30, 3mo).

Ok, digging into this a little further. Looks like contribs changes for 7 and 30, but not for 3mo. Nothing else seems to change though. And the tooltips say that it's over the selected time period.

https://github.com/open-sauced/app/assets/34313413/1586efaa-fe04-4bc2-b9da-b680c80131b6

jpmcb commented 8 months ago

Yeah, what's in code is inconsistent with what users expect or what we should be surfacing. We'll need some guidance from product on what these cards should be showing. We may also be removing these existing cards in favor of these KPIs implemented here: https://github.com/open-sauced/app/pull/2455

... but I'm not sure what the direction for all that should be.

BekahHW commented 8 months ago

@jpmcb didn't realize that I had an issue opened that related back in Nov: #2178

brandonroberts commented 8 months ago

We discussed this during our engineering meeting and will remove the spam metrics from repo insights for now and replace with a more relevant metric. Spam was only used more prominently during Hacktoberfest.

I would recommend opening a separate issue to remove it and closing this one.