Skip to content

Commit f440e3e

Browse files
committed
Unifies work access by DOI or internal ID
Allows accessing work landing pages and related functionalities using either the DOI or the internal database ID, simplifying the URL structure. Improves robustness when accessing works, especially those without DOIs. Introduces a utility function to handle identifier resolution. Also fixes a UI issue where extraction options were not initially visible in Geoextent. Updates UI tests to align with changes in map popup content and navigation.
1 parent fa83c46 commit f440e3e

17 files changed

+368
-140
lines changed

optimap/sitemaps.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ def location(self, item):
1919
"""Return the URL path for a work (without domain)."""
2020
# Return only the path, not the full URL (Django's sitemap adds domain)
2121
if item.doi:
22-
return reverse("optimap:article-landing", args=[item.doi])
22+
return reverse("optimap:work-landing", args=[item.doi])
2323
else:
2424
return f"/work/{item.id}/"
2525

tests-ui/test_admin_content.py

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,30 @@
2323

2424

2525
def get_work_from_api():
26-
"""Helper function to get a work (id, doi) from the API instead of database."""
26+
"""
27+
Helper function to get a work (id, doi) from the API instead of database.
28+
Returns the first published work found, preferring works with DOI.
29+
Returns identifier field that can be used in URLs (either DOI or ID).
30+
"""
2731
response = requests.get('http://localhost:8000/api/v1/works/', timeout=5)
2832
if response.status_code == 200:
2933
data = response.json()
3034
if data.get('results') and len(data['results']) > 0:
3135
work = data['results']['features'][0]
32-
return {'id': work.get('id'), 'doi': work.get('properties').get('doi'), 'title': work.get('properties').get('title')}
33-
36+
work_id = work.get('id')
37+
work_doi = work.get('properties').get('doi')
38+
work_title = work.get('properties').get('title')
39+
40+
# Use DOI as identifier if available, otherwise use ID
41+
identifier = work_doi if work_doi else str(work_id)
42+
43+
return {
44+
'id': work_id,
45+
'doi': work_doi,
46+
'title': work_title,
47+
'identifier': identifier # Can be used in /work/<identifier>/ URLs
48+
}
49+
return None
3450

3551
class AdminContentVisibilityTests(TestCase):
3652
"""Test that admin-only content is properly restricted."""
@@ -144,9 +160,12 @@ def test_work_landing_page_anonymous_no_admin_buttons(self):
144160

145161
# Get work from API instead of database
146162
work_data = get_work_from_api()
163+
if not work_data or not work_data.get('identifier'):
164+
self.skipTest('No works available via API')
147165

148166
try:
149-
start_chrome(f'localhost:8000/work/{work_data["doi"]}/', headless=True)
167+
# Use the unified identifier (DOI or ID)
168+
start_chrome(f'localhost:8000/work/{work_data["identifier"]}/', headless=True)
150169
driver = get_driver()
151170

152171
# Wait for page to load

tests-ui/test_article_landing.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ def setUp(self):
2222
)
2323

2424
def test_page_renders_with_map_when_geometry_present(self):
25-
url = reverse("optimap:article-landing", args=[self.pub_with_geom.doi])
25+
url = reverse("optimap:work-landing", args=[self.pub_with_geom.doi])
2626
resp = self.client.get(url)
2727
self.assertEqual(resp.status_code, 200)
2828
# feature_json is provided
@@ -32,7 +32,7 @@ def test_page_renders_with_map_when_geometry_present(self):
3232
self.assertContains(resp, 'id="mini-map"', count=1)
3333

3434
def test_page_hides_map_when_no_geometry(self):
35-
url = reverse("optimap:article-landing", args=[self.pub_no_geom.doi])
35+
url = reverse("optimap:work-landing", args=[self.pub_no_geom.doi])
3636
resp = self.client.get(url)
3737
self.assertEqual(resp.status_code, 200)
3838
# feature_json omitted or None
@@ -42,7 +42,7 @@ def test_page_hides_map_when_no_geometry(self):
4242
self.assertNotContains(resp, 'id="mini-map"')
4343

4444
def test_unknown_doi_returns_404(self):
45-
url = reverse("optimap:article-landing", args=["10.9999/missing"])
45+
url = reverse("optimap:work-landing", args=["10.9999/missing"])
4646
self.assertEqual(self.client.get(url).status_code, 404)
4747

4848
class ArticleLandingTests(TestCase):
@@ -61,7 +61,7 @@ def setUp(self):
6161
)
6262

6363
def test_page_renders_with_map_when_geometry_present(self):
64-
url = reverse("optimap:article-landing", args=[self.pub_with_geom.doi])
64+
url = reverse("optimap:work-landing", args=[self.pub_with_geom.doi])
6565
resp = self.client.get(url)
6666
self.assertEqual(resp.status_code, 200)
6767
# feature_json is provided
@@ -71,7 +71,7 @@ def test_page_renders_with_map_when_geometry_present(self):
7171
self.assertContains(resp, 'id="mini-map"', count=1)
7272

7373
def test_page_hides_map_when_no_geometry(self):
74-
url = reverse("optimap:article-landing", args=[self.pub_no_geom.doi])
74+
url = reverse("optimap:work-landing", args=[self.pub_no_geom.doi])
7575
resp = self.client.get(url)
7676
self.assertEqual(resp.status_code, 200)
7777
# feature_json omitted or None
@@ -81,5 +81,5 @@ def test_page_hides_map_when_no_geometry(self):
8181
self.assertNotContains(resp, 'id="mini-map"')
8282

8383
def test_unknown_doi_returns_404(self):
84-
url = reverse("optimap:article-landing", args=["10.9999/missing"])
84+
url = reverse("optimap:work-landing", args=["10.9999/missing"])
8585
self.assertEqual(self.client.get(url).status_code, 404)

tests-ui/test_geoextent.py

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -185,12 +185,22 @@ def test_remote_form_validation(self):
185185
kill_browser()
186186

187187
def test_extraction_options_visible(self):
188-
"""Test that all extraction options are visible."""
188+
"""Test that all extraction options are visible after clicking on the options link."""
189189
try:
190190
start_chrome(f'{self.base_url}/geoextent/', headless=True)
191191

192+
self.assertFalse(Text("Bounding box").exists())
193+
self.assertFalse(Text("Bounding box").exists())
194+
self.assertFalse(Text("Time box").exists())
195+
self.assertFalse(Text("Convex hull").exists())
196+
self.assertFalse(Text("Place name").exists())
197+
self.assertFalse(Text("Output format").exists())
198+
self.assertFalse(Text("Gazetteer service").exists())
199+
200+
click("Extraction options")
201+
192202
# Check all option labels exist
193-
self.assertTrue(Text("Bounding box").exists())
203+
wait_until(lambda: Text("Bounding box").exists(), timeout_secs=5)
194204
self.assertTrue(Text("Time box").exists())
195205
self.assertTrue(Text("Convex hull").exists())
196206
self.assertTrue(Text("Place name").exists())
@@ -225,14 +235,10 @@ def test_documentation_section_visible(self):
225235
finally:
226236
kill_browser()
227237

228-
def test_footer_link_navigates_to_geoextent(self):
229-
"""Test that clicking geoextent link in footer navigates to the page."""
238+
def test_sitemap_link_navigates_to_geoextent(self):
239+
"""Test that clicking geoextent link on the user sitemap navigates to the page."""
230240
try:
231-
start_chrome(f'{self.base_url}/', headless=True)
232-
233-
# Scroll to footer
234-
driver = get_driver()
235-
driver.execute_script("window.scrollTo(0, document.body.scrollHeight);")
241+
start_chrome(f'{self.base_url}/pages/', headless=True)
236242

237243
# Click geoextent link in footer
238244
click("Geoextent")
@@ -241,6 +247,7 @@ def test_footer_link_navigates_to_geoextent(self):
241247
wait_until(lambda: Text("Geoextent extraction").exists(), timeout_secs=5)
242248

243249
# Check URL changed
250+
driver = get_driver()
244251
self.assertIn("geoextent", driver.current_url)
245252

246253
finally:

tests-ui/test_ui_map.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,16 @@ def test_map_page(self):
1111
self.assertTrue(S('#map').exists())
1212

1313
leaflet_paths = find_all(S('path.leaflet-interactive'))
14-
self.assertEqual(len(leaflet_paths), 2) # has two polygons on the map
14+
self.assertGreater(len(leaflet_paths), 0) # has geometries on the map
1515
for path in leaflet_paths:
16-
self.assertEqual(path.web_element.get_attribute('stroke'), '#3388ff')
16+
self.assertEqual(path.web_element.get_attribute('stroke'), '#158F9B')
1717

1818
click(leaflet_paths[0])
1919

20-
wait_until(lambda: Text('Visit Article').exists())
20+
wait_until(lambda: Text('View work details').exists())
2121

22-
self.assertIn('title of article', S('div.leaflet-popup-content').web_element.text)
22+
# we do not know which popup, so we cannot test for much
23+
self.assertIn('Visit work', S('div.leaflet-popup-content').web_element.text)
2324

2425
get_driver().save_screenshot(r'tests-ui/screenshots/map_popup.png')
2526

tests/test_work_landing_page.py

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -419,3 +419,129 @@ def test_api_viewset_queryset_filtering(self):
419419
self.assertIn(self.pub_testing, queryset)
420420
self.assertIn(self.pub_withdrawn, queryset)
421421
self.assertIn(self.pub_harvested, queryset)
422+
423+
424+
class MultipleIdentifierAccessTest(TestCase):
425+
"""Tests for accessing works by various identifier types (DOI, ID, future: handle)."""
426+
427+
def setUp(self):
428+
self.client = Client()
429+
430+
# Create test source
431+
self.source = Source.objects.create(
432+
name="Test Journal",
433+
url_field="https://example.com/oai",
434+
homepage_url="https://example.com/journal",
435+
issn_l="1234-5678"
436+
)
437+
438+
# Create a work with DOI
439+
self.work_with_doi = Work.objects.create(
440+
title="Work with DOI",
441+
abstract="This work has a DOI",
442+
url="https://example.com/work1",
443+
status="p",
444+
doi="10.1234/test-doi",
445+
publicationDate=now() - timedelta(days=30),
446+
geometry=GeometryCollection(Point(12.4924, 41.8902)),
447+
source=self.source
448+
)
449+
450+
# Create a work without DOI
451+
self.work_without_doi = Work.objects.create(
452+
title="Work without DOI",
453+
abstract="This work has no DOI",
454+
url="https://example.com/work2",
455+
status="p",
456+
publicationDate=now() - timedelta(days=20),
457+
geometry=GeometryCollection(Point(13.4050, 52.5200)),
458+
source=self.source
459+
)
460+
461+
def test_access_work_by_doi(self):
462+
"""Test that a work can be accessed by its DOI."""
463+
response = self.client.get(f'/work/{self.work_with_doi.doi}/')
464+
self.assertEqual(response.status_code, 200)
465+
self.assertContains(response, self.work_with_doi.title)
466+
self.assertContains(response, self.work_with_doi.doi)
467+
468+
def test_access_work_by_internal_id(self):
469+
"""Test that a work can be accessed by its internal ID."""
470+
response = self.client.get(f'/work/{self.work_with_doi.id}/')
471+
self.assertEqual(response.status_code, 200)
472+
self.assertContains(response, self.work_with_doi.title)
473+
474+
def test_access_work_without_doi_by_id(self):
475+
"""Test that a work without DOI can be accessed by its internal ID."""
476+
response = self.client.get(f'/work/{self.work_without_doi.id}/')
477+
self.assertEqual(response.status_code, 200)
478+
self.assertContains(response, self.work_without_doi.title)
479+
# Should not show DOI link since work has no DOI
480+
self.assertNotContains(response, 'https://doi.org/')
481+
482+
def test_work_with_doi_prefers_doi_identifier(self):
483+
"""Test that DOI is detected correctly even if ID could also match."""
484+
# DOI starts with "10." so should be detected as DOI
485+
response = self.client.get(f'/work/{self.work_with_doi.doi}/')
486+
self.assertEqual(response.status_code, 200)
487+
self.assertContains(response, self.work_with_doi.title)
488+
489+
def test_numeric_id_resolves_correctly(self):
490+
"""Test that numeric IDs are handled correctly."""
491+
# Access by numeric ID
492+
response = self.client.get(f'/work/{self.work_with_doi.id}/')
493+
self.assertEqual(response.status_code, 200)
494+
self.assertContains(response, self.work_with_doi.title)
495+
496+
def test_invalid_identifier_returns_404(self):
497+
"""Test that an invalid identifier returns 404."""
498+
response = self.client.get('/work/99999999/') # Non-existent ID
499+
self.assertEqual(response.status_code, 404)
500+
501+
response = self.client.get('/work/10.9999/nonexistent/') # Non-existent DOI
502+
self.assertEqual(response.status_code, 404)
503+
504+
def test_work_without_doi_title_format(self):
505+
"""Test that works without DOI have correct title format (no DOI in parentheses)."""
506+
response = self.client.get(f'/work/{self.work_without_doi.id}/')
507+
self.assertEqual(response.status_code, 200)
508+
509+
# Extract the title tag content
510+
content = response.content.decode('utf-8')
511+
512+
# Should have title without DOI
513+
self.assertIn(f'<title>{self.work_without_doi.title} - OPTIMAP</title>', content)
514+
515+
# Should NOT have DOI in parentheses
516+
self.assertNotIn(f'({self.work_without_doi.title})', content)
517+
518+
def test_template_handles_null_doi(self):
519+
"""Test that the template correctly handles works with null DOI."""
520+
response = self.client.get(f'/work/{self.work_without_doi.id}/')
521+
self.assertEqual(response.status_code, 200)
522+
523+
# Should have title
524+
self.assertContains(response, self.work_without_doi.title)
525+
526+
# Should NOT have DOI section
527+
self.assertNotContains(response, '<strong>DOI:</strong>')
528+
529+
# JavaScript variables should handle empty DOI
530+
self.assertContains(response, 'const doi = ""')
531+
self.assertContains(response, 'const useIdUrls = true')
532+
533+
def test_url_encoded_doi_works(self):
534+
"""Test that URL-encoded DOIs are properly decoded and work."""
535+
# Create work with DOI that has special characters
536+
work = Work.objects.create(
537+
title="Work with special DOI",
538+
abstract="Test",
539+
status="p",
540+
doi="10.1234/test-doi/with-slash",
541+
source=self.source
542+
)
543+
544+
# Django's URL routing should handle this automatically
545+
response = self.client.get(f'/work/{work.doi}/')
546+
self.assertEqual(response.status_code, 200)
547+
self.assertContains(response, work.title)

works/models.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ def permalink(self) -> str | None:
145145
if not getattr(self, "doi", None):
146146
return None
147147
base = settings.BASE_URL.rstrip("/")
148-
rel = reverse("optimap:article-landing", args=[self.doi])
148+
rel = reverse("optimap:work-landing", args=[self.doi])
149149
return f"{base}{rel}"
150150
permalink.short_description = "Permalink"
151151

works/sitemaps.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ def location(self, item):
1919
"""Return the URL path for a work (without domain)."""
2020
# Return only the path, not the full URL (Django's sitemap adds domain)
2121
if item.doi:
22-
return reverse("optimap:article-landing", args=[item.doi])
22+
return reverse("optimap:work-landing", args=[item.doi])
2323
else:
2424
return f"/work/{item.id}/"
2525

works/templates/contribute.html

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,16 @@
22

33
{% block title %}Contribute Geolocation Data | {% endblock %}
44

5+
{% block head %}
6+
{{ block.super }}
7+
<style>
8+
/* Hide search bar on contribute page (no functionality here) */
9+
#navbar-search-container {
10+
display: none !important;
11+
}
12+
</style>
13+
{% endblock %}
14+
515
{% block content %}
616

717
<div class="container-fluid locate-container">
@@ -134,11 +144,11 @@ <h5 class="card-title">
134144

135145
{% if request.user.is_authenticated %}
136146
{% if work.doi %}
137-
<a href="{% url 'optimap:article-landing' work.doi %}" class="btn btn-primary btn-sm">
147+
<a href="{% url 'optimap:work-landing' work.doi %}" class="btn btn-primary btn-sm">
138148
<i class="fas fa-map-marked-alt"></i> Contribute metadata
139149
</a>
140150
{% else %}
141-
<a href="{% url 'optimap:publication-by-id' work.id %}" class="btn btn-primary btn-sm">
151+
<a href="{% url 'optimap:work-landing' work.id %}" class="btn btn-primary btn-sm">
142152
<i class="fas fa-map-marked-alt"></i> Contribute metadata
143153
</a>
144154
{% endif %}

works/templates/feed_page.html

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ <h1 class="py-2">{{ region.name }} - Latest works</h1>
3535
<div class="card-body">
3636
<h5 class="card-title">
3737
{% if work.doi %}
38-
<a href="{% url 'optimap:article-landing' work.doi %}" rel="noopener">
38+
<a href="{% url 'optimap:work-landing' work.doi %}" rel="noopener">
3939
{{ work.title|truncatechars:120 }}
4040
</a>
4141
{% elif work.url %}
@@ -77,7 +77,7 @@ <h5 class="card-title">
7777
</small>
7878

7979
{% if work.doi %}
80-
<a href="{% url 'optimap:article-landing' work.doi %}" class="btn btn-primary btn-sm">
80+
<a href="{% url 'optimap:work-landing' work.doi %}" class="btn btn-primary btn-sm">
8181
<i class="fas fa-map-marked-alt"></i> View work's page
8282
</a>
8383
{% elif work.url %}

0 commit comments

Comments
 (0)