Skip to content

[FIXIES #13030] Endpoint to delete a resource's thumbnail#13032

Merged
giohappy merged 4 commits intomasterfrom
ISSUE_13030
Apr 15, 2025
Merged

[FIXIES #13030] Endpoint to delete a resource's thumbnail#13032
giohappy merged 4 commits intomasterfrom
ISSUE_13030

Conversation

@Gpetrak
Copy link
Copy Markdown
Member

@Gpetrak Gpetrak commented Apr 4, 2025

This PR is created according to this issue #13030 . The endpoint will delete the thumbnail image from disk and the corresponding database fields: thumbnail_field and thumbnail_path

Checklist

Reviewing is a process done by project maintainers, mostly on a volunteer basis. We try to keep the overhead as small as possible and appreciate if you help us to do so by completing the following items. Feel free to ask in a comment if you have troubles with any of them.

For all pull requests:

  • Confirm you have read the contribution guidelines
  • You have sent a Contribution Licence Agreement (CLA) as necessary (not required for small changes, e.g., fixing typos in the documentation)
  • Make sure the first PR targets the master branch, eventual backports will be managed later. This can be ignored if the PR is fixing an issue that only happens in a specific branch, but not in newer ones.

The following are required only for core and extension modules (they are welcomed, but not required, for contrib modules):

  • There is a ticket in https://github.com/GeoNode/geonode/issues describing the issue/improvement/feature (a notable exemption is, changes not visible to end-users)
  • The issue connected to the PR must have Labels and Milestone assigned
  • PR for bug fixes and small new features are presented as a single commit
  • Commit message must be in the form "[Fixes #<issue_number>] Title of the Issue"
  • PR title must be in the form "[Fixes #<issue_number>] Title of the PR"
  • New unit tests have been added covering the changes, unless there is an explanation on why the tests are not necessary/implemented
  • This PR passes all existing unit tests (test results will be reported by travis-ci after opening this PR)
  • This PR passes the QA checks: black geonode && flake8 geonode
  • Commits changing the settings, UI, existing user workflows, or adding new functionality, need to include documentation updates
  • Commits adding new texts do use gettext and have updated .po / .mo files (without location infos)

Submitting the PR does not require you to check all items, but by the time it gets merged, they should be either satisfied or inapplicable.

@cla-bot cla-bot bot added the cla-signed CLA Bot: community license agreement signed label Apr 4, 2025
@Gpetrak Gpetrak requested a review from giohappy April 4, 2025 07:08
@Gpetrak Gpetrak self-assigned this Apr 4, 2025
@giohappy giohappy requested review from mattiagiupponi and removed request for giohappy April 4, 2025 14:35
@giohappy
Copy link
Copy Markdown
Contributor

giohappy commented Apr 4, 2025

@Gpetrak before changing the failing tests please ensure that it's not a temporary failure. Those tests worked fine until now.
In case speak with @mattiagiupponi, which is also the reviewer for your PR.

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 8, 2025

Codecov Report

Attention: Patch coverage is 92.59259% with 4 lines in your changes missing coverage. Please review.

Project coverage is 68.00%. Comparing base (050aa97) to head (90b6cf2).
Report is 16 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #13032      +/-   ##
==========================================
+ Coverage   67.98%   68.00%   +0.02%     
==========================================
  Files         980      980              
  Lines       59299    59352      +53     
  Branches     6931     6934       +3     
==========================================
+ Hits        40312    40363      +51     
  Misses      17339    17339              
- Partials     1648     1650       +2     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Gpetrak
Copy link
Copy Markdown
Member Author

Gpetrak commented Apr 8, 2025

@Gpetrak before changing the failing tests please ensure that it's not a temporary failure. Those tests worked fine until now.
In case speak with @mattiagiupponi, which is also the reviewer for your PR.

@giohappy finally the test_dataset_permissions test was broken and with the help of @mattiagiupponi the test was fixed and merged to master. Now the tests pass.

@giohappy giohappy self-requested a review April 8, 2025 22:14
Copy link
Copy Markdown
Contributor

@giohappy giohappy left a comment

Choose a reason for hiding this comment

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

@Gpetrak please add a test for the new endpoint

@Gpetrak
Copy link
Copy Markdown
Member Author

Gpetrak commented Apr 9, 2025

@giohappy I just added the tests for the delete_thumbnail endpoint

@giohappy giohappy self-requested a review April 10, 2025 09:00
@Gpetrak Gpetrak changed the title [FIXIES 13030] Endpoint to delete a resource's thumbnail [FIXIES #13030] Endpoint to delete a resource's thumbnail Apr 12, 2025
@Gpetrak Gpetrak added the feature A new feature to be added to the codebase label Apr 12, 2025
@Gpetrak Gpetrak added this to the 5.0.0 milestone Apr 12, 2025
# Resource does not exist
invalid_url = reverse("base-resources-delete-thumbnail", kwargs={"resource_id": 9999})
response = self.client.post(invalid_url)
self.assertEqual(response.status_code, 404)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hi @Gpetrak i guess it could be worth to add a test where a user, tries to delete the thumbnail of a resource which does not belong to it.
like user bobby tries to delete the thumbnail of a resource owner by user norm. I suppose the user should not have the right to do it

@giohappy giohappy requested a review from mattiagiupponi April 15, 2025 12:49
@giohappy giohappy merged commit ddb0317 into master Apr 15, 2025
18 checks passed
@giohappy giohappy deleted the ISSUE_13030 branch April 15, 2025 12:49
@mattiagiupponi mattiagiupponi removed this from the 5.0.0 milestone Oct 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed CLA Bot: community license agreement signed feature A new feature to be added to the codebase

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement the API thumbnail endpoint method to delete the thumbnail for a resource

3 participants