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.

Sign up for GitHub

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

Jump to bottom

[FIX] Showing thumbnails on files list #23301

Merged
merged 9 commits into from Dec 12, 2022
Merged

[FIX] Showing thumbnails on files list #23301

merged 9 commits into from Dec 12, 2022

Conversation

ostjen
Copy link
Contributor

@ostjen ostjen commented Sep 28, 2021

before
image

after
image

Proposed changes (including videos or screenshots)

Issue(s)

Steps to test or reproduce

Further comments

@ostjen ostjen requested a review from ggazzo September 28, 2021 12:09
@ostjen ostjen changed the title [IMPROVE] list only files that are not thumbnails [FIX] list only files that are not thumbnails Sep 28, 2021
sampaiodiego
sampaiodiego requested changes Sep 29, 2021
View reviewed changes
app/api/server/v1/im.js Outdated Show resolved Hide resolved
sampaiodiego
sampaiodiego requested changes Oct 20, 2021
View reviewed changes
Copy link
Member

@sampaiodiego sampaiodiego left a comment

Choose a reason for hiding this comment

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

I liked the migration idea, but I we need few indexes to make it run fine in big servers.

server/startup/migrations/v241.ts Outdated Show resolved Hide resolved
server/startup/migrations/v241.ts Outdated Show resolved Hide resolved
sampaiodiego
sampaiodiego requested changes Nov 1, 2021
View reviewed changes
app/api/server/v1/groups.js Outdated Show resolved Hide resolved
app/api/server/v1/im.js Outdated Show resolved Hide resolved
server/startup/migrations/v243.ts Outdated Show resolved Hide resolved
app/models/server/models/Uploads.js Outdated Show resolved Hide resolved
server/startup/migrations/v243.ts Outdated Show resolved Hide resolved
@CLAassistant
Copy link

CLAassistant commented Dec 17, 2021 •
edited

CLA assistant check
All committers have signed the CLA.

@lgtm-com
Copy link

lgtm-com bot commented Sep 30, 2022

This pull request introduces 7 alerts when merging a5d8202 into 0e6bc45 - view on LGTM.com

new alerts:

  • 7 for Property access on null or undefined

@ostjen
Copy link
Contributor Author

ostjen commented Oct 3, 2022

nice

pierre-lehnen-rc
pierre-lehnen-rc previously requested changes Oct 13, 2022
View reviewed changes
apps/meteor/server/startup/migrations/v282.ts Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Oct 13, 2022 •
edited

Codecov Report

Merging #23301 (72c6ae2) into develop (461a8e2) will increase coverage by 0.11%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #23301      +/-   ##
===========================================
+ Coverage    42.04%   42.15%   +0.11%     
===========================================
  Files          818      818              
  Lines        17772    17772              
  Branches      2003     2003              
===========================================
+ Hits          7472     7492      +20     
+ Misses       10030    10001      -29     
- Partials       270      279       +9
Flag Coverage Δ
e2e 42.15% <ø> (+0.11%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

sampaiodiego
sampaiodiego requested changes Oct 14, 2022
View reviewed changes
Copy link
Member

@sampaiodiego sampaiodiego left a comment

Choose a reason for hiding this comment

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

we're also missing tests.

apps/meteor/server/models/raw/Uploads.ts Outdated Show resolved Hide resolved
apps/meteor/server/startup/migrations/v282.ts Outdated Show resolved Hide resolved
@sampaiodiego sampaiodiego mentioned this pull request Oct 18, 2022
Closed
@sampaiodiego sampaiodiego requested a review from a team as a code owner November 10, 2022 19:52
@LucianoPierdona LucianoPierdona added this to the 6.0.0 milestone Nov 29, 2022
@ggazzo ggazzo removed the communityPR label Dec 6, 2022
@dionisio-bot dionisio-bot bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: needs QA labels Dec 12, 2022
@sampaiodiego sampaiodiego changed the title [FIX] list only files that are not thumbnails [FIX] Showing thumbnails on files list Dec 12, 2022
@kodiakhq kodiakhq bot merged commit 3cf1c80 into develop Dec 12, 2022
@kodiakhq kodiakhq bot deleted the uploaded_files_fix branch December 12, 2022 20:08
sampaiodiego added a commit that referenced this pull request Jan 25, 2023
@ostjen @carlosrodrigues94 @sampaiodiego
[FIX] Showing thumbnails on files list (#23301)
623fcce 
Co-authored-by: Carlos Rodrigues <51969060+carlosrodrigues94@users.noreply.github.com>
Co-authored-by: Diego Sampaio <8591547+sampaiodiego@users.noreply.github.com>
@sampaiodiego sampaiodiego mentioned this pull request Feb 17, 2023
Merged
@sampaiodiego sampaiodiego mentioned this pull request Mar 9, 2023
Merged
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sampaiodiego sampaiodiego sampaiodiego approved these changes

@ggazzo ggazzo Awaiting requested review from ggazzo

@pierre-lehnen-rc pierre-lehnen-rc Awaiting requested review from pierre-lehnen-rc

Assignees
No one assigned
Labels
squad: team-collab stat: QA skipped stat: ready to merge PR tested and approved waiting for merge
Projects
None yet
Milestone
6.0.0
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@ostjen @CLAassistant @sampaiodiego @pierre-lehnen-rc @dudanogueira @ggazzo @alvaropmello @LucianoPierdona @carlosrodrigues94