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

Correct predicate for admin unit classification #79

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

piemonkey
Copy link
Contributor

Overview

Discovered when working on creating decision templates in RB.

connected issues and PRs:

#77

Setup

N/A

How to test/reproduce

Sparql query for the data or look in fct?

Challenges/uncertainties

This hasn't been thoroughly tested as the functionality that required it isn't actually needed. Maybe it should have been a draft?

Checks PR readiness

  • UI: works on smaller screen sizes
  • UI: feedback for any loading/error states
  • Check cancel/go-back flows
  • Check database state correct when deleting/updating (especially regarding relationships)
  • changelog
  • no new deprecations

@abeforgit
Copy link
Member

@piemonkey can you just elaborate a tiny bit on what the problem was/is? I have a vague sense but not entirely sure

@piemonkey
Copy link
Contributor Author

It's been a while... Honestly, I can't do much better than the changeset and linking to a rocketchat conversation.

From that conversation, the problem was when I was trying to get RB to correctly display the right decision types for the administrative unit of the logged in user. It later turned out that we didn't want to do this, so I removed the front-end part, but thought I'd correct the data model anyway as I had already done that locally. So this PR is essentially just correcting some incorrect data that could potentially cause problems later if not fixed.

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.

2 participants