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

Fixed bug with userdisplay and latestrelease #2053

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

IR96334
Copy link
Member

@IR96334 IR96334 commented Mar 18, 2025

No description provided.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Ubuntu seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

1 similar comment
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Ubuntu seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@IR96334 IR96334 marked this pull request as ready for review March 18, 2025 16:22
{` ${formatDateString(file.createdAt.toString())}`}
{sortedAssociatedReleases.length > 0 && (
<Typography textOverflow='ellipsis' overflow='hidden' variant='caption' sx={{ mb: 2 }}>
Added by <UserDisplay dn={sortedAssociatedReleases[sortedAssociatedReleases.length - 1].createdBy} />{' '}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Added by" about a file is different to who created one of the associated releases. Also how would this work if a file wasn't associated to any releases?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To my knowledge the user who uploaded the file is only stored in the release. This means if a file in the future was able to be uploaded separately to a release, I don't think you could display the release of the uploader until the file store holds the name of the uploader.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we don't store who uploaded a file, but we do store who created a release but those are different things. As a result I don't think we can have "Added by" on a file because we don't have that information.

@IR96334 IR96334 requested a review from JR40159 March 19, 2025 10:34
</Typography>
)}
</Stack>
<Stack direction={{ sm: 'column', md: 'row' }} spacing={0.5} alignItems='center' justifyContent='flex-start'>
Copy link
Member

@ARADDCC002 ARADDCC002 Mar 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to break these two bits up as they're the same sentence. Instead I would remove this <Stack /> and replace it with:

Uploaded on <span style={{fontWeight: 'bold'}}>{${formatDateString(file.createdAt.toString())}}

@IR96334 IR96334 requested a review from ARADDCC002 March 19, 2025 13:44
</Typography>
</Stack>
</Stack>
<Typography variant='caption' sx={{ mb: 2 }}>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need this padding at the bottom

@ARADDCC002
Copy link
Member

Moved to #2060

@ARADDCC002 ARADDCC002 closed this Mar 19, 2025
@IR96334 IR96334 reopened this Mar 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants