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

Update Build Logs view to show alert when build logs are over 180 days #4379

Closed
1 task done
apburnes opened this issue Jan 29, 2024 · 2 comments · Fixed by #4402
Closed
1 task done

Update Build Logs view to show alert when build logs are over 180 days #4379

apburnes opened this issue Jan 29, 2024 · 2 comments · Fixed by #4402
Assignees

Comments

@apburnes
Copy link
Contributor

apburnes commented Jan 29, 2024

When a user navigates to build logs over 180 days, display an alert that build logs over 180 days are deleted based on platform policy

Acceptance Criteria

  • Shows alert when over 180 days
@svenaas
Copy link
Contributor

svenaas commented Feb 8, 2024

After looking at how this works, I'd like to ask your thoughts, @apburnes and @sknep:

Current UX: Right now, the build logs screen shows the "This build does not have any build logs." message initially, replacing that with the actual build logs once they load — if they load.

Intended UX: I was going to add an alternate message — "Builds more than 180 days old are deleted according to platform policy." — displayed if build.completedAt is more than 180 days ago.

But the SiteBuildLogs component does not otherwise have or need the build object itself, just the build id and — if they exist and are returned by the API — the logs.

Option 1: Don't check the date at all and just include the 180 day text at the end of the existing message.

Option 2: Have the SiteBuildLogs component retrieve the build just so it can check the date.

But CommitSummary component, which is included on this page, already retrieves and uses the build as a state object. So I think these sorts of things become possible:

Option 3: Modify CommitSummary to take a callback function through which it can pass the build date to SiteBuildLogs once this is retrieved.

Option 4: Move the build state and retrieval up to the parent component so that it can be shared.

However, either of these options would change the way CommitSummary is used so the build tasks code would be affected as well.

What do you think? Option 1 is easy, but affects the UX by including the 180 day text in cases where it may not be relevant. Options 2, 3, and 4 allow for a more precise message, at the cost of changed complexity.

@sknep
Copy link
Contributor

sknep commented Feb 8, 2024

I like options 3 and 4 the best. Having the default text even on load be "there are no logs" isn't great. Ideally there'd be a "Loading logs..." until we know whether logs would have been found, and then it would change to

  • no logs found for this build (for cancelled builds)
  • logs for builds older than 180 days are not available
  • [the actual logs in the black box]

Would be interested in looking closer at this with you if you like.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants