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

By-laws on LawLibrary #1432

Closed
wants to merge 3 commits into from
Closed

By-laws on LawLibrary #1432

wants to merge 3 commits into from

Conversation

nickmwangemi
Copy link
Contributor

@nickmwangemi nickmwangemi commented Aug 10, 2023

image
image
image

@nickmwangemi nickmwangemi force-pushed the bylaws-lawlibrary branch 2 times, most recently from 5be9387 to 1dbbb6d Compare August 10, 2023 13:24
@nickmwangemi nickmwangemi marked this pull request as ready for review August 15, 2023 05:54
navbar_link = "legislation/municipal"

def get(self, *args, **kwargs):
self.municipality = get_object_or_404(Locality, code=kwargs["code"])
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a reminder that this is exactly what LocalityLegislationListView does, using the word locality rather than municipality. So this isn't necessary.


class LocalityLegislationListView(LegislationListView):
template_name = "liiweb/locality_legislation_list.html"
navbar_link = "legislation/locality"
provincial_codes = PROVINCIAL_CODES
Copy link
Contributor

Choose a reason for hiding this comment

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

This can't be done in liiweb, because provinces are a lawlibrary thing only.

def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs)
localities = Locality.objects.all()
context["province_groups"] = list(chunks(localities, 2))
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to stay here, but rename province_groups to locality_groups.

@@ -0,0 +1,36 @@
{% if locality_type == "province" %}
<nav class="nav nav-tabs mb-3 border-bottom">
Copy link
Contributor

Choose a reason for hiding this comment

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

The two if blocks are identical except for whether subleg is included. Rather remove the if block, and then pass in a flag from the View object to indicate whether the subleg should be shown.

<a href="{% url 'locality_legislation_list' document.locality.code %}">{{ document.locality.name }}</a>
</li>
</ol>
{% if document.locality.code in PROVINCIAL_CODES %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Only the first part of the breadcrumb changes, the rest is the same. So put the if block only around the part that changes, otherwise you're duplicating code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants