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

Fix issue where config has to be treated as a list of values #18

Closed
simonw opened this issue Nov 20, 2023 · 1 comment
Closed

Fix issue where config has to be treated as a list of values #18

simonw opened this issue Nov 20, 2023 · 1 comment
Labels
enhancement New feature or request
Milestone

Comments

@simonw
Copy link
Collaborator

simonw commented Nov 20, 2023

Thanks to this code:

config = post_vars._data.copy()
config.pop("csrftoken", None)

The config dictionary ends up full of things like {"template": ["this is the template"]} which means I have to write code like this:

template = env.from_string(config["template"][0])
output_column = config["output_column"][0]

I did this because I wanted to support the case of a multi-select style interface for selecting a list of items here:

class ConfigForm(Form):
columns = MultiCheckboxField("Columns", choices=choices)
return ConfigForm
async def enrich_batch(
self,
db: Database,
table: str,
rows: List[dict],
pks: List[str],
config: dict,
job_id: int,
):
columns = config.get("columns") or []

But there should be a better way of doing this - I want config to mostly be single valued keys, with lists only for the keys that need it.

@simonw simonw added the enhancement New feature or request label Nov 20, 2023
@simonw
Copy link
Collaborator Author

simonw commented Nov 20, 2023

Figured out the right pattern for this - it needs this change:

-    config = post_vars._data.copy()
-    config.pop("csrftoken", None)
+    config = {field.name: field.data for field in form}

Plus this change to that multi column selecting form:

-class MultiCheckboxField(SelectField):
+class MultiCheckboxField(SelectMultipleField):
     widget = ListWidget(prefix_label=False)
     option_widget = CheckboxInput()

@simonw simonw closed this as completed in 211c2c6 Nov 20, 2023
@simonw simonw added this to the First alpha milestone Nov 20, 2023
simonw added a commit that referenced this issue Nov 20, 2023
config now has {"values": "like this"} instead of {"values": ["like this"]}, refs #18
simonw added a commit to datasette/datasette-enrichments-jinja that referenced this issue Nov 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant