Skip to content

Support for relational custom field conditions #16

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

Merged
merged 7 commits into from
Dec 20, 2021

Conversation

bayangan1991
Copy link
Contributor

@bayangan1991 bayangan1991 commented Dec 16, 2021

Converted to poetry and blackfied the codebase in 32ac507
Added support for relational custom field conditions in 5489be3
Rebuilt example app with tests b220077

PBI : WF-4247

Relational Custom Field Conditions

In a workforce custom field, you can specify conditions on foreign keys to prevent rendering on the django forms. This conditions currently only work on fields directly on the model.

This PR introduces the ability for a condition to walk ORM relational model to apply conditions.

For example, with a custom field on an asset with the following model:
Client -> Property -> Asset

Currently, a field can only be conditionally shown based on the property it is assigned to:
key=property value=1
and for multiple properties
key=property value=1,2,3...

But, if you wanted a field to be visible for all properties on a client, you would have to list and maintain every property

With this PR you can walk the ORM model between ForeignKeys using __. So to add a condition for the field only be displayed on Client 1:
key=property__client value=1

This would limit the custom fields visibility for any assets assigned to properties on Client 1, without having to specify each property.

Copy link
Member

@jarekwg jarekwg left a comment

Choose a reason for hiding this comment

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

Code looks sound!
Great stuff poetrying/blacking it at the same time. Nice that you've done it in a separate commit. I think your original approach of submitting that as a separate PR altogether was even better, but this works too.

The current amount of tests in this repo is pretty dispappointing heh.. Would you mind adding a couple, just for this nested work you've added? (Also not fussed if we just merge this and add the tests in the WF repo instead..)

PS: shoot me your PyPI username, and I'll add you to the team so you can publish a new release of this package.

@bayangan1991
Copy link
Contributor Author

Hey Jarek,
Tests have been added to workforce, will shoot you the link in private if you want to look over them.

@bayangan1991 bayangan1991 self-assigned this Dec 17, 2021
@jarekwg jarekwg requested a review from DamianHeard December 20, 2021 00:33
@DamianHeard
Copy link

For what it is worth I would very much have preferred two PRs here.

pyproject.toml Outdated
target-version = ['py39']
include = '\.pyi?$'
experimental_string_processing = true
force_exclude = '''

Choose a reason for hiding this comment

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

Non-blocking comment: I would be tempted to remove these exclusions. There's only one model and there's no url file or migrations.

Copy link

@DamianHeard DamianHeard left a comment

Choose a reason for hiding this comment

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

I'm going to tick this PR but suggest you probably want to bump the char limit on the key or the feature will have limited use.

@bayangan1991 bayangan1991 merged commit 02fdf6c into master Dec 20, 2021
@bayangan1991 bayangan1991 deleted the feature/relational-conditions branch December 20, 2021 04:35
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