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 result retrieval issues #241

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

abought
Copy link

@abought abought commented Jan 31, 2025

🦑 Happy to break this up into >1 PRs. It represents a set of tweaks I made to support a particular demo use case.

Purpose

  • Fix an issue where search result links could sometimes generate "subject not found" links for detail pages (due to mixing of not-inverse quote_plus / unquote operations)
    • This manifested when a search index subject id name contained spaces
  • Improve handling of fields in DGPF
    • Explicitly document all three syntaxes that were supported previously, and the need to specify the special template fields explicitly in fields config
    • Fix mismatch between doc and project dependencies that prevented local doc build on Py3.13 (cgi module was removed, and old django versions used by docs tried to import it)
    • Add missing unit test for existing (field name, alias) mode
    • Allow (field_name, alias) mode to work with dotted path syntax (item.citation.title). Previously it only worked with top level keys. Now it can fetch a field that is inside a nested object
      • I did not update the get exact field name syntax for dotted paths, on the grounds that template variables shouldn't have . in the name

TODO

  • On Monday I'll test this inside my own custom portal to verify functionality in a real world test case.
  • Verify that doc dep changes work in CI for old versions of python (5 warnings exist but they predate this PR- see comments below)

At present there isn't a good way to specify "path to function". This is slightly inconvenient for 12-factor style of passing in index config from outside the container.
(render always returns HttpResponse)
* URLs for the detail page were generated via `quote_plus`, but evaluated using `unquote`. If a subject ID contained spaces, the search page would generate URLs that could not be resolved to a detail page.

*  Remove reference to old functionality in docstring, per https://github.com/globus/django-globus-portal-framework/blob/main/CHANGELOG.md#030---2019-03-07
Improves handling of search results when values are under a nested key
@abought abought marked this pull request as draft January 31, 2025 22:59
@abought abought marked this pull request as ready for review February 3, 2025 21:09
@abought
Copy link
Author

abought commented Feb 3, 2025

With new dependencies, the doc build is generating 5 warnings. But it seems those warnings were also being generated previously under old versions, eg this December CI run:
https://github.com/globus/django-globus-portal-framework/actions/runs/12320265953/job/34389140427

Might be a good idea to review known issues before merging, to make sure these aren't new breakages. I can brush up on my sphinx to dig in if needed.

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.

1 participant