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

Load More button is visible when there are no more data to load on the Audit Logs page #2390

Open
3 tasks done
unrenamed opened this issue Oct 13, 2024 · 11 comments
Open
3 tasks done
Labels
Bug Something isn't working Needs Approval Needs approval from Unkey

Comments

@unrenamed
Copy link
Contributor

Preliminary Checks

Reproduction / Replay Link (Optional)

No response

Issue Summary

On the Audit Logs page, the Load More button is displayed even when all available log entries have already been loaded. This can cause confusion for users, as clicking the button does not load any additional data, suggesting there might be more logs when there aren't any.

Steps to Reproduce

  1. Navigate to the Audit Logs page.
  2. Scroll down to the bottom of the list.
  3. Load additional logs by clicking the Load More button.
  4. Continue loading until all logs are loaded.
  5. Observe that the Load More button remains visible even though there are no more logs to load.

Expected behavior

Once all available audit logs have been loaded, the Load More button should disappear or be disabled, indicating to the user that no more logs are available for loading.

Other information

No response

Screenshots

No response

Version info

- OS:
- Node:
- npm:
@unrenamed unrenamed added Bug Something isn't working Needs Approval Needs approval from Unkey labels Oct 13, 2024
@adityaraj-09
Copy link
Contributor

/assign

Copy link

oss-gg bot commented Oct 13, 2024

This issue is not part of oss.gg hackathon. Please pick a different one or start with a side quest

@0xJaskeerat
Copy link

0xJaskeerat commented Oct 15, 2024

I guess this is an issue of Virtualization , I have worked on similar issue in another project as well
@chronark and @perkinsjr Please review this once, if you find it worthy enough , please add a oss.gg label and I would love to contribute to it on firsthand

@chronark
Copy link
Collaborator

What should happen when there are new audit logs created since they last refreshed though?

When they refreshed, there were no additional ones, but after a few seconds for example there could be new ones

@unrenamed
Copy link
Contributor Author

unrenamed commented Oct 30, 2024

@chronark these are really good points! My thoughts on how we can approach this:

Option 1: Polling for New Logs
Implement a polling mechanism that checks for new audit logs at set intervals. If new logs are detected, enable or display the "Load More" button.

Option 2: Loading State
When the button is clicked, a user sees loading state rather than seeing that the page is "reloaded". If no new logs are found after the request, provide feedback to the user, such as a message saying "No more logs available." right above the button.

Option 3: Loading State (simple)
Keep everything as it is, just show a toast with a message that "No more logs available.".

@chronark
Copy link
Collaborator

we should coordinate this with what @ogzhanolguncu is working on for our /logs page, the experience should be the same for the end user

the logs page will have a "live" mode, where we do background polling via react-query, I guess we should do the same here
but also let the user manually refresh and show them "no new logs found"

so I guess a combination of (1) + (2) would be nice

@ogzhanolguncu
Copy link
Contributor

/logs page is not quite ready, but here is what we have there right now. Initial data is fetched from RSC, then passed to a client component, then react-query kicks in and checks and refreshes regularly every 2 seconds. We can easily turn this into a 'Load more' button and let users fetch the data at their will. This is the most intuitive solution that came to my mind, and I'm open to suggestions.

@unrenamed
Copy link
Contributor Author

unrenamed commented Oct 30, 2024

@ogzhanolguncu sounds good to me 👍

the main issue on this page rn is lack of feedback for a user.

let users fetch the data at their will

yep, and show a message if no new logs are found.

@ogzhanolguncu
Copy link
Contributor

Yeah, once we are done with /logs, /audit page should be trivial. If you have a working solution as a temp fix I think we can move forward with that then replace it with actual design.

@unrenamed
Copy link
Contributor Author

unrenamed commented Oct 30, 2024

lol. either I'm missing something or the bug is actually much bigger than I initially thought 😅

It's not really about the Load More button being visible when there is nothing to load.
I found this in the code:

 <div className="w-full mt-8">
  <Link href={buildHref({ before: logs.at(-1)?.time })} prefetch>
    <Button
      size="block"
      disabled={!hasMoreLogs}
      variant={hasMoreLogs ? "secondary" : "disabled"}
    >
      {hasMoreLogs ? "Load more" : "No more logs"}
    </Button>
  </Link>
</div>

Apparently, indicating that no more logs are available for loading has been supported for months.

I dig dipper and found that this page does not work as it might be expected for several reasons:

  1. Incorrect page URL on Load More click
    The code builds this URL: return "/audit?${searchParams.toString()}";. It should be /audit/unkey_mutations.
  2. Logs are not filtered by time
    props.searchParams.before is not used for filtering logs when querying from the DB
  3. Logs are not being "loaded more", they are rather being "loaded instead"
    When you look closer, the bucket logs list is being overwritten instead of being extended. Each query replaces the previous data with new.

Here is a link to the file: apps/dashboard/app/(app)/audit/[bucket]/page.tsx

Given this, a temporary fix may not be sufficient. It likely needs a full re-examination and rewrite, potentially from scratch, ideally following the new design standards used on the /logs page.

@chronark @ogzhanolguncu

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Needs Approval Needs approval from Unkey
Projects
None yet
Development

No branches or pull requests

5 participants