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

In listing views, I want to see locality information if applicable #1692

Merged
merged 2 commits into from
Jan 22, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions africanlii/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
"zimlii",
]
PEACHJAM["SEARCH_JURISDICTION_FILTER"] = True # noqa
PEACHJAM["MULTIPLE_JURISDICTIONS"] = True # noqa
PEACHJAM["MULTIPLE_LOCALITIES"] = True # noqa

# The slugs of the taxonomy roots that are treated as federated indexes
FEDERATED_DOC_INDEX_ROOTS = ["case-indexes"]
Expand Down
2 changes: 2 additions & 0 deletions lawlibrary/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
JAZZMIN_SETTINGS["site_header"] = "Lawlibrary" # noqa
JAZZMIN_SETTINGS["site_brand"] = "Lawlibrary.org.za" # noqa

PEACHJAM["MULTIPLE_LOCALITIES"] = True # noqa


TEMPLATES[0]["OPTIONS"]["context_processors"].append( # noqa
"lawlibrary.context_processors.lawlibrary"
Expand Down
2 changes: 2 additions & 0 deletions peachjam/context_processors.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ def general(request):
"SUPPORT_EMAIL": settings.PEACHJAM["SUPPORT_EMAIL"],
"PEACHJAM_SETTINGS": pj_settings(),
"CURRENT_LANGUAGE": language,
"MULTIPLE_JURISDICTIONS": settings.PEACHJAM["MULTIPLE_JURISDICTIONS"],
"MULTIPLE_LOCALITIES": settings.PEACHJAM["MULTIPLE_LOCALITIES"],
# this object will be injected into Javascript to provide configuration settings to the Javascript app
"PEACHJAM_JS_CONFIG": {
"appName": settings.PEACHJAM["APP_NAME"],
Expand Down
2 changes: 2 additions & 0 deletions peachjam/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,8 @@
"CITATOR_API_KEY": os.environ.get("CITATOR_API_KEY"),
"EXTRA_SEARCH_INDEXES": [],
"SEARCH_JURISDICTION_FILTER": False,
"MULTIPLE_JURISDICTIONS": False,
"MULTIPLE_LOCALITIES": False,
}

PEACHJAM["ES_INDEX"] = os.environ.get("ES_INDEX", slugify(PEACHJAM["APP_NAME"]))
Expand Down
12 changes: 8 additions & 4 deletions peachjam/templates/peachjam/_document_table.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
<thead>
<tr>
<th scope="col" style="width: 70%">{% trans 'Title' %}</th>
{% if doc_table_show_jurisdiction %}
Copy link
Contributor

Choose a reason for hiding this comment

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

What was the logic to determine the value of this variable previously?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's set only in africanlii to show the jurisdiction of taxonomies alone. But if we're showing jurisdiction across the site, I didn't think it'd matter.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed. Can you remove the logic that sets it in views.py or wherever?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Screenshot 2024-01-22 at 11 53 32

<th scope="col">{% trans 'Country' %}</th>
{% if MULTIPLE_JURISDICTIONS or MULTIPLE_LOCALITIES %}
<th scope="col">{% trans 'Jurisdiction' %}</th>
{% endif %}
{% if doc_table_show_author %}<th scope="col"></th>{% endif %}
{% if doc_table_show_court %}<th scope="col"></th>{% endif %}
Expand All @@ -19,8 +19,12 @@
<td>
<a href="{{ document.get_absolute_url }}">{{ document.title }}</a>
</td>
{% if doc_table_show_jurisdiction %}
<td>{% jurisdiction_icon document %}&nbsp;{{ document.jurisdiction }}</td>
{% if MULTIPLE_JURISDICTIONS and MULTIPLE_LOCALITIES %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Not quite:

  1. if MULTIPLE_JURISDICTIONS
    a. show document.country
    b. if document.locality, show it
  2. elif MULTIPLE_LOCALITIES
    a. if document.locality, show it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In the case of MULTIPLE_JURISDICTIONS, do you want the locality showing after the jurisdiction or before? Or should it be a separate column?

Copy link
Contributor

Choose a reason for hiding this comment

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

Same column, but after. Separate with a https://en.wikipedia.org/wiki/Interpunct ·

eg. South Africa · Gauteng

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is there a difference between if document.locality, show it and document.locality|default_if_none:''? Either way, a document without locality won't display anything

Copy link
Contributor

Choose a reason for hiding this comment

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

Correct, no difference, except that you don't want the middot if there is no locality.

<td>
{% jurisdiction_icon document %}&nbsp;{{ document.locality|default_if_none:'' }}&nbsp;{{ document.jurisdiction|default_if_none:'' }}
</td>
{% elif MULTIPLE_LOCALITIES %}
<td>{{ document.locality|default_if_none:'' }}</td>
{% endif %}
{% if doc_table_show_author %}<td>{{ document.author|default_if_none:'' }}</td>{% endif %}
{% if doc_table_show_court %}<td>{{ document.court|default_if_none:'' }}</td>{% endif %}
Expand Down
Loading