Skip to content

Conversation

@EkaterinaPoddubskaya
Copy link
Contributor

This PR resolves issue #117

- `ISIC_NEW_API_URL` to the absolute URL of the ISIC API root
- this must include a trailing slash
- e.g. this value may be `https://api-sandbox.isic-archive.com/api/v2/` for testing
- e.g. this value may be `https://api.isic-archive.com/api/v2/` for testing
Copy link
Member

Choose a reason for hiding this comment

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

👍

@neuroelf
Copy link
Contributor

Hello @EkaterinaPoddubskaya and @danlamanna -- when I visit the pre-deploy page on an incognito (no history) window, and then select under "TBP TYPE" : "3D: XP", I see a lot of "Not found" messages pop up on my screen (see screenshot). Can either of you take a look what might be the cause?

Gallery_4449d472_TBP_TYPE_3D-_XP_Not_Found_2025-06-17

@EkaterinaPoddubskaya
Copy link
Contributor Author

The reason is that we don't have lesion data coming from the server for some of the pictures

@neuroelf
Copy link
Contributor

Hello @EkaterinaPoddubskaya -- thanks for the quick investigation and reply! Much appreciated :)

Since "lesion_id" is not a mandatory field (not all images are associated with a lesion), the UI should not consider that an error (Not Found). Is there a way to make this a "soft fail" (where the user will not see a message)?

@EkaterinaPoddubskaya
Copy link
Contributor Author

Hello @neuroelf ! Yes, implementing a soft fail is possible. I'll be looking into it tomorrow and will update you accordingly.

@neuroelf
Copy link
Contributor

Hello @danlamanna and @EkaterinaPoddubskaya -- you two probably know github quite a bit better (than I do, I mean)... Given that some files have been changed in both this and another PR, does github automatically refactor this by applying diffs, or is it important in which order I apply the PRs? Or maybe you, @EkaterinaPoddubskaya, have made the changes in the overlapping files (package.json and imagesFilters.js in sources/models) in both PRs?

@neuroelf
Copy link
Contributor

Otherwise, given this PR has only 4 files changed... Would it make sense to add these changes to the other PR (#121)?

@EkaterinaPoddubskaya
Copy link
Contributor Author

Hello @neuroelf, while the order of PRs might be important, it’s not a major issue since I can resolve any conflicts if they arise. However, I would recommend keeping PRs separate to maintain clear and focused logic.

@neuroelf
Copy link
Contributor

@EkaterinaPoddubskaya -- OK, I will then merge the PRs in numeric order (unless you suggest a different order).

Can you let me know if/when the "soft fail" is (can be) implemented? I would really want to avoid gallery visitors seeing that error message :)

@EkaterinaPoddubskaya
Copy link
Contributor Author

@neuroelf, numeric order should work fine. The "soft fail" is already implemented, but I need some time to test it properly before pushing. My guess is you'll be able to see it in this branch by tomorrow morning :)

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Jun 18, 2025

Deploying isic-gallery with  Cloudflare Pages  Cloudflare Pages

Latest commit: 97ada7f
Status: ✅  Deploy successful!
Preview URL: https://fc264d71.isic-gallery.pages.dev
Branch Preview URL: https://add-technological-attributes.isic-gallery.pages.dev

View logs

@neuroelf neuroelf marked this pull request as ready for review June 18, 2025 18:55
@neuroelf neuroelf merged commit 61ab395 into master Jun 19, 2025
3 checks passed
@neuroelf neuroelf deleted the add-technological-attributes-facets branch June 19, 2025 11:44
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