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

Feature/detail enhance municipality button behavior #52

Merged
merged 9 commits into from
Jul 13, 2023

Conversation

aliokan
Copy link
Member

@aliokan aliokan commented Jul 11, 2023

This PR contains:

  • model type fixes
  • Update relationship between agenda-item, session and governing-body (Related to the BREAKING CHANGE API Fix(resources): agenda-item can have many sessions app-burgernabije-besluitendatabank#17)
  • Use administrative-unit.location.label everywhere to resolve municipality name instead of administrative-unit.name
  • Remove the not working filtering :has: from query
  • Redirect users to the agendaItems list, filtered specifically for the selected municipality when clicking on municipality Button

Copy link
Contributor

@Denperidge Denperidge left a comment

Choose a reason for hiding this comment

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

A few things are unclear to my tired brain, but nothing that would or should stop this PR!

app/models/agenda-item.ts Show resolved Hide resolved
app/models/governing-body.ts Show resolved Hide resolved
app/models/session.ts Show resolved Hide resolved
@aliokan aliokan requested a review from Windvis July 12, 2023 08:02
}));

return session;
return this.store.findRecord('session', session_id, {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we might have had the same idea for simplifying this request, I think I also did something similar in one of the PR's woopss
But that's neither here nor there, we'll cross that bridge when we get there! Love the streamlining you did

Copy link
Contributor

@Denperidge Denperidge left a comment

Choose a reason for hiding this comment

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

New changes, second approve!

Copy link
Contributor

@Windvis Windvis left a comment

Choose a reason for hiding this comment

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

Looks good, only some remarks about the async relationship handling 👍.

app/models/agenda-item.ts Outdated Show resolved Hide resolved
app/routes/detail.ts Outdated Show resolved Hide resolved
app/controllers/map.ts Outdated Show resolved Hide resolved
- Improve accessing async relationships in `AgendaItemModel` and `DetailRoute`
- Improve typing in `AgendaItemModel` and `DetailRoute`
- start vote code cleaning
@aliokan aliokan requested a review from Windvis July 12, 2023 21:22
@aliokan
Copy link
Member Author

aliokan commented Jul 12, 2023

@Windvis ready for the next round! 🏓

Copy link
Contributor

@Windvis Windvis left a comment

Choose a reason for hiding this comment

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

👍

@Windvis Windvis merged commit 0035c6e into master Jul 13, 2023
@Windvis Windvis deleted the feature/detail-enhance-municipality-button-behavior branch July 13, 2023 07:20
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.

3 participants