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

fix(file)!: dont handle case where existing file URL was re-attached in get_full_path #18584

Closed
wants to merge 2 commits into from

Conversation

sagarvora
Copy link
Collaborator

@sagarvora sagarvora commented Oct 26, 2022

The problem described in #17250 will only get solved for new attachments. It will continue for existing attachments.

That's okay, because get_url can be unreliable:

  • doesn't give you all domains
  • doesn't work if site URL was changed
  • works better only when called as part of a request

@sagarvora sagarvora requested review from shariquerik, ankush and a team and removed request for a team and surajshetty3416 October 26, 2022 13:41
@github-actions github-actions bot added the add-test-cases Add test case to validate fix or enhancement label Oct 26, 2022
@sagarvora sagarvora removed the request for review from a team October 26, 2022 13:41
@sagarvora sagarvora removed the add-test-cases Add test case to validate fix or enhancement label Oct 26, 2022
@sagarvora sagarvora changed the title fix(file): dont handle case where existing file URL was re-attached in get_full_path fix(file)!: dont handle case where existing file URL was re-attached in get_full_path Oct 28, 2022
@sagarvora
Copy link
Collaborator Author

Same backport labels as #17250 have been added.

@codecov
Copy link

codecov bot commented Oct 28, 2022

Codecov Report

Merging #18584 (3c308b6) into develop (d35aa6c) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #18584      +/-   ##
===========================================
- Coverage    63.07%   63.06%   -0.01%     
===========================================
  Files          747      747              
  Lines        67410    67402       -8     
  Branches      5996     5996              
===========================================
- Hits         42516    42505      -11     
- Misses       21434    21437       +3     
  Partials      3460     3460              
Flag Coverage Δ
server-mariadb 67.16% <87.50%> (-0.02%) ⬇️
server-postgres 67.18% <87.50%> (-0.01%) ⬇️

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

@stale
Copy link

stale bot commented Nov 5, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed within 3 days if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing.

@stale stale bot added the inactive label Nov 5, 2022
@stale stale bot closed this Nov 9, 2022
@sagarvora sagarvora removed the inactive label Nov 9, 2022
@sagarvora sagarvora reopened this Nov 9, 2022
@github-actions github-actions bot added the add-test-cases Add test case to validate fix or enhancement label Nov 9, 2022
@stale
Copy link

stale bot commented Nov 19, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed within 3 days if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing.

@stale stale bot added the inactive label Nov 19, 2022
@sagarvora
Copy link
Collaborator Author

@ankush Let's close this if we aren't looking to fix it as of now.

@stale stale bot removed the inactive label Nov 19, 2022
@stale
Copy link

stale bot commented Nov 29, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed within 3 days if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing.

@stale stale bot added the inactive label Nov 29, 2022
@sagarvora sagarvora closed this Nov 29, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
add-test-cases Add test case to validate fix or enhancement backport version-14-hotfix backport to version 14 inactive
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants