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

Configurable Forms for flows #679

Draft
wants to merge 19 commits into
base: main
Choose a base branch
from
Draft

Configurable Forms for flows #679

wants to merge 19 commits into from

Conversation

isuruf
Copy link
Contributor

@isuruf isuruf commented Oct 1, 2019

cc @lukeolson

A form description looks like https://github.com/isuruf/relate-sample/blob/91226393241cfed6457db533addfb2da6e83f645/forms/instant.yml
There are 5 types of fields. Text, Integer, Float, Choice, Hidden.
template_in and template_out are mandatory fields and they can be either Hidden or Text. If Hidden the values are fixed, but if Text, the user who fills the form can fill them.
If template_out=flows/asd.yml then the actual output would be flows/asd_20190930_022148_311577.yml where the time is appended.

There's a type for a form which is only flow for now. If there's a announce field and set to true and the type is a flow, then an InstantFlowRequest is created.

Flow created looks like

{% with id="20190930_022148_311577",
        field1="1",
        field2="A",
        template_in="flows/instant_flow.jinja",
        template_out="flows/instant_flow.yml",
        announce="True",
        created_time="2019-09-30 @ 02:21" %}
{% include "flows/instant_flow.jinja" %}
{% endwith %}

The templated flow template_in can use the form variables by their id and also have access to created_time and id. created_time is useful for instant flows that @lukeolson mentioned.

Let me know what you think.

@isuruf
Copy link
Contributor Author

isuruf commented Oct 7, 2019

@inducer, let me know what you think. I'll add tests if you think there shouldn't be any major re-write.

@codecov
Copy link

codecov bot commented Oct 13, 2019

Codecov Report

Merging #679 into master will decrease coverage by 0.48%.
The diff coverage is 74.29%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #679      +/-   ##
==========================================
- Coverage   96.56%   96.08%   -0.49%     
==========================================
  Files          45       46       +1     
  Lines       11215    11459     +244     
  Branches     2084     2137      +53     
==========================================
+ Hits        10830    11010     +180     
- Misses        298      361      +63     
- Partials       87       88       +1
Impacted Files Coverage Δ
course/content.py 96.7% <100%> (ø) ⬆️
relate/urls.py 100% <100%> (ø) ⬆️
course/constants.py 100% <100%> (ø) ⬆️
course/validation.py 99.83% <100%> (+0.01%) ⬆️
course/forms.py 65.21% <65.21%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 22643e6...4dafc64. Read the comment docs.


class FormsBase(SingleCourseTestMixin, MockAddMessageMixing, TestCase):

initial_commit_sha = "f3e9d31a61714e759a6ea12b900b173accb753f5"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@inducer, I used this commit from inducer/relate-sample#12 which has flows/instant_flow.jinja file and it is included by the created flow using {% include 'flows/instant_flow.jinja' %}, but when I try this in tests the templating engine can't find this file. How can I make jinja2 see this file?

Base automatically changed from master to main March 8, 2021 02:15
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.

1 participant