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

Updated - feat: UI - Performer scraper result list with image and basic data #3499

Conversation

StashPRs
Copy link
Contributor

@StashPRs StashPRs commented Mar 1, 2023

This is the same as #2697, updated for the codebase as it sits currently and improved upon slightly. I'd definitely appreciate a code review on this, it's been a few years since I've messed around with React.

From the original PR:

As the title says, but also added toast and error messages.
closes #2695

UI Changes

To the performer scraper query modal:

query modal

To the performer scraper confirmation modal:

confirmation modal

@DogmaDragon DogmaDragon added improvement Something needed tweaking. ui Issues related to UI labels Mar 1, 2023
@WithoutPants WithoutPants added this to the Version 0.20.0 milestone Mar 1, 2023
@StashPRs
Copy link
Contributor Author

StashPRs commented Mar 1, 2023

Latest 3 commits fixup the eslint issues I had

@WithoutPants
Copy link
Collaborator

Getting all images and encoding them as base64 is the wrong approach in my opinion. The stash-box implementation returns the URLs to the images only. We should probably return the direct URLs, and they should be loaded only as needed by the UI.

I think that this PR should be split up so that we can solve these issues separately:

  • one PR for the query dialog to show age, gender and image
  • one PR to support multiple images in the performer scrape dialog - stash-box results already return multiple images, so this can be tested with stash-box
  • finally, one PR to change the backend to support multiple images from non-stash-box scrapers. We may need to figure out a way to ensure that the referrer header isn't set when getting each image.

I won't close this PR yet, but I won't be merging it with the current design.

@StashPRs
Copy link
Contributor Author

StashPRs commented Mar 9, 2023

Thanks for the review. I can work with that :)

@TgSeed
Copy link
Contributor

TgSeed commented Mar 9, 2023

Getting all images and encoding them as base64 is the wrong approach in my opinion. The stash-box implementation returns the URLs to the images only. We should probably return the direct URLs, and they should be loaded only as needed by the UI.

I'm not sure but as far as i remember, i have once again talked or wrote regarding this somewhere, maybe in the code comments!

That's the right approach in my opinion!
Just how it requests data directly from the device/system that has Stash running, itself should also fetch everything that can relate to that.

Requesting data with one Device/IP and other things such as assets from another Device/IP can simply leak IP and/or other things, it may not even be what the user really want.
Well, It barely happen, but imagine working in an office with a monitored network and connecting to your home stash, they may not know what are you doing with that IP/Domain, but they will definitely see you connected likely to "cdn.brazzers.com" or such.
That's not my case, I'm just so careful about my data leakages and privacy.

@WithoutPants
Copy link
Collaborator

Requesting data with one Device/IP and other things such as assets from another Device/IP can simply leak IP and/or other things, it may not even be what the user really want. Well, It barely happen, but imagine working in an office with a monitored network and connecting to your home stash, they may not know what are you doing with that IP/Domain, but they will definitely see you connected likely to "cdn.brazzers.com" or such. That's not my case, I'm just so careful about my data leakages and privacy.

Good point. I kind of touched the surface of that with the referrer stuff. I think the solution is as follows:

  • add a server route that accepts an encoded URL and returns the data, by requesting the URL from the backend. For example: <server:port>/scrape_data/<encoded URL>. The server handles this request by decoding the encoded URL, performing the http request using its normal scraper http client, and returns the data, basically like a pass through.
  • if a scrape result returns a single image, it can get the data and return it as a base64 chunk, but if there's multiple URLs, then encode them into the above format and return those. From the client's point of view it won't make a difference what the URL.

@WithoutPants WithoutPants removed this from the Version 0.20.0 milestone Mar 21, 2023
@StashPRs StashPRs closed this Jul 30, 2023
@StashPRs StashPRs deleted the feat/ui/performer_scraper_result_list_with_image branch July 30, 2023 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Something needed tweaking. ui Issues related to UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] Performer scraper result list with image and more
4 participants