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

How to handle Django models.TextChoice with Djelm ? #99

Closed
YievCkim opened this issue May 12, 2024 · 15 comments
Closed

How to handle Django models.TextChoice with Djelm ? #99

YievCkim opened this issue May 12, 2024 · 15 comments

Comments

@YievCkim
Copy link

YievCkim commented May 12, 2024

Hi !

I'm having a bit of a struggle using django's choices field from Elm.

From django I have this model:

class StoryStatus(models.TextChoices):
    CREATED = "created"
    IN_PROGRESS = "in progress"
    SUSPENDED = "suspended"
    CANCELED = "canceled"
    FINISHED = "finished"

class UserStory(models.Model):
    status = models.CharField(
        max_length=20,
        choices=StoryStatus,
        default=StoryStatus.CREATED,
    )

If I try to use a CustomType there is no mean for djelm to distinguish them since there are all strings and it seems it chooses the first when converting.

So I had to use simple StringFlag and convert string from Elm type directly from elm code.

For that, I created an explicit Elm type:

type Status
    = Created
    | OnGoing
    | Cancelled
    | Suspended
    | Finished
    | Unknown

and an utility function to convert string from django to the elm proper variant:

statusFromString : String -> Status
statusFromString s =
    case s of
        "created" ->
            Created

        "suspended" ->
            Suspended

        "canceled" ->
            Cancelled

        "in progress" ->
            OnGoing

        "finished" ->
            Finished

        _ ->
            Unknown

Note I need an additional variant in case of an unknown choice potentially provided by django.
(This could be useful for detecting a change in the possible choices on the django side that would not have been reflected in the front-end code.)

I do understand that this is a tricky point to tackle, that said.

Maybe we could have an EnumFlag something like that.

Any idea ?

For full example see these commits:

https://github.com/MikHulk/epics/commit/307b9b821aa5bb116b1415994910b126ef60a778

https://github.com/MikHulk/epics/commit/8aee32611c8a35e8190642935cb3e296ee9de47e

@YievCkim YievCkim changed the title Django enum type to Elm Django models.TextChoice to Elm May 12, 2024
@YievCkim YievCkim changed the title Django models.TextChoice to Elm How to handle Django models.TextChoice with Djelm ? May 12, 2024
@Confidenceman02
Copy link
Owner

Confidenceman02 commented May 12, 2024

Hi @MikHulk!

Thanks for this issue! It's a juicy one! Love it!

f I try to use a CustomType there is no mean for djelm to distinguish them since there are all strings and it seems it
chooses the first when converting.

Yes!! You are completely correct! If you give a custom type variants that all resolve to StringFlag then all it does is check if it is a string and it will validate. The first one would win!

There are a bunch of ways we could solve for this I'll just put my thoughts here and would love to hear from you what you think feels the right way, or your ideas on how you would solve it.

LiteralFlag

it might be handy if we had a LiteralFlag, then it would actually validate against a literal string. This would also automatically allow for a fall back! Something like the following:

CustomTypeFlag(
    [ 
        ("Created", LiteralFlag("created")),
        ("Ongoing", LiteralFlag("in progress")),
        ("Cancelled", LiteralFlag("cancelled")),
        ("Suspended", LiteralFlag("suspended")),
        ("Finished", LiteralFlag("finished")),
        ("Unknown", String()), # <-- Catch all
    ]

Extend StringFlag

We could add some config to StringFlag to allow for matching a literal:

CustomTypeFlag(
    [
        ("Created", StringFlag(literal="created"))
        ...
    ]
)

TextChoices flag

We could create a utility for the django TextChoices class, much like ModelChoiceField. Djelm then would do the internal work to create the CustomType:

TextChoicesFlag(StoryStatus)

@Confidenceman02
Copy link
Owner

Confidenceman02 commented May 12, 2024

To solve your issue currently you could disambiguate the value that comes through. It's a tiny bit hacky but it would make djelm resolve the values correctly. Consider the following!

For demonstration purposes it assumes a very simple context, but you could translate this to the actual context you have.

In your view:

def some_view():

  user_story = UserStory.objects.get(pk=1)

  context = dict([(user_story.status, user_story.status)]) # <--  { "created": "created"} 


# flags
CustomTypeFlag(
    [ 
        ("Created", ObjectFlag({"created": StringFlag()})),
        ...
    ]

This doesn't solve for a catch all unfortunately, it would cause an error. Maybe that's ok though?

Because you are using TextChoices my understanding is you are limiting your choices via an enumeration. So if django produces something not in that enumeration, something may have gone wrong somewhere?

In fact in a form, Django I believe would reject anything that is not in that enumeration during form validation.

Let me know if this hacky suggestion gets you some results!

@YievCkim
Copy link
Author

YievCkim commented May 12, 2024

Hi !

This doesn't solve for a catch all unfortunately, it would cause an error. Maybe that's ok though?

Yes I just add this on Elm side because without Elm complains on the case of in statusFromString. I am going to try right now your suggestion I think it should work.

For your suggestion for a new flag my first idea was TextChoicesFlag indeed (I called it EnumFlag but idea it's the same). Just keep in mind that it exists an IntegerChoice too and that is possible to subclasse choice. We could have something like:

ChoiceFlag(StoryStatus, StringFlag())

Or maybe it's possible to deduce that directly from the class passed to ChoiceFlag.

That said I like the idea of LiteralFlag or even the possibility of configuring StringFlag.
(BTW, I think it could be a good idea to have the possibility to choose a name for the model name in Elm with that. But it's an other discussion.)
As I'm only reading data from the server at the moment, I don't have enough hindsight to know what would be the best choice both for reading data from the server and for writing it.

@Confidenceman02
Copy link
Owner

Confidenceman02 commented May 12, 2024

Just keep in mind that it exists an IntegerChoice too and that is possible to subclasse choice.

Yea that's a good point! I imagine it would be possible to deduce, python is great like that.

I also like the idea of LiteralFlag, because djelm uses pydantic under the hood, it matches perfectly with the Literal pydantic type.

The only thing that bothers me is that LiteralFlag doesn't map to any Elm primitive. My vision for Flags is that they Map to Elm primitives as closely as possible. So maybe adding config to StringFlag is actually the better option!

@YievCkim
Copy link
Author

I have just tried your workaround and I obtain this model:

type Epic_Stories__Status__
    = Created Epic_Stories__Status__Created___
    | OnGoing Epic_Stories__Status__OnGoing___
    | Suspended Epic_Stories__Status__Suspended___
    | Cancelled Epic_Stories__Status__Cancelled___
    | Finished Epic_Stories__Status__Finished___


type alias Epic_Stories__Status__Created___ =
    { created : String }

type alias Epic_Stories__Status__OnGoing___ =
    { in_progress : String }

type alias Epic_Stories__Status__Suspended___ =
    { suspended : String }

type alias Epic_Stories__Status__Cancelled___ =
    { canceled : String }

type alias Epic_Stories__Status__Finished___ =
    { finished : String }

In some way it works but it's a bit hard to pattern match it . Idealy it would be very cool to get this:

type Status
    = Created
    | OnGoing
    | Cancelled
    | Suspended
    | Finished

I'll try to see how the generation works tomorrow. To get an idea of the feasibility of one of the 3 solutions you have proposed. I think it would be really nice to have this possibility.

@Confidenceman02
Copy link
Owner

Confidenceman02 commented May 12, 2024

Yep that looks right!

The pattern matching as I see it would be as follows:

doSomethingWithStatus : Epic_Stories__Status__ ->  something
doSomethingWithStatus status =
    case status of 
        Created _ ->
            -- Do something with created
        ...

This doesn't seem too bad to me, but I would love to understand what about it feels tricky for you. Perhaps we might be able to improve it! 😄

The parameter is probably not important which is why I _ it.

In regards to the naming, Djelm protects all top level type declarations, otherwise it would be incredibly easy to get name clashes like the following:

type alias Status =
    { something : String
    , status : Status
    }

type Status
    = Something

The result of this protection means the Model module type names are not concise, so you can get some pretty interesting names!

If this becomes a problem you could always just map the type to something more concise.

type Status
    = Created
    ...

mapToStatus : Epic_Stories__Status__ -> Status
mapToStatus storiesStatus =
    case storiesStatus of
        Model.Created _ ->
            Created
        ....

To me though, it doesn't seem like this adds much other than a prettier type name.

Let me know what you think!

@YievCkim
Copy link
Author

YievCkim commented May 12, 2024

having Status or Epic_Stories__Status_ is ok. I should have written:

type Epic_Stories__Status_
    = Created
    | OnGoing
    | Cancelled
    | Suspended
    | Finished

It's mainly the fact of having compound variants that's a bit cumbersome.

In my case I need to use these types and sometimes instantiate them from the frontend. For example, here.

One might also need to use this type in another composite type. In my case, I need them in Msg.

That said, to be perfectly clear, this would be a nice improvement. And of course it's perfectly possible to work with Djelm as it is now.

@Confidenceman02
Copy link
Owner

In my case I need to use these types and sometimes instantiate them from the frontend.

Oh! Yes I can totally understand from an instantiation point of view, that is cumbersome indeed!

I 100% agree that it would be a nice improvement, and I'm totally onboard with making it happen.

I'm going to think about a nice way to achieve this! It's a good enhancement!

@YievCkim
Copy link
Author

That's great ! Please, let me know about your investigations. 😄

@Confidenceman02
Copy link
Owner

Ok I have thought about this and I think I have a good idea.

Step 1 String literals

One thing that I think is most important is that the Elm decoding should exactly match the pydantic validation.

This means that if we are declaring a string literal validation in python via pydantic, then the Elm decoders should also express that same constraint.

The reason this is important is that the Elm programs should be able to receive JSON from anywhere and still behave exactly the same. Put simply, Elm decoding should be mirror python validation, always! (Currently it does).

The Elm analogue for string literal validation roughly translates as follows:

Decode.string |> Decode.andThen matchLitereal

The Djelm compiler makes it possible to easily create these functions inline. We would use the op module for this.

Step 2 CustomType constructors with no values

I'm still thinking about this and I will have a clearer idea after we get string literals on how we could express it.

Im flying home from Spain on the weekend so im going to see if I can smash it out in the air! Wish me luck!

@YievCkim
Copy link
Author

YievCkim commented May 17, 2024

Have a nice and fruitful trip from Spain !

I believe you're on the right path with LiteralFlag.

The reason this is important is that the Elm programs should be able to receive JSON from anywhere

100% agree with that I already have needed to use the model to validate JSON received from the DRF API . So I think this point is important.

The main issue is Django represents choice with two "kind" of value the python value used in python code and its representation (an example). DRF returns the representation by default but from python code the enum variant are used unde their "real" name. So I am not sure about how to handle this on Elm side.
I have to say, the more I think about it, the more puzzled I am. Since you know how Djelm works internally I guess you have a better idea. I guess we should only consider the string representation coming from python on Elm side and map them to a sum type in Elm.

@Confidenceman02
Copy link
Owner

I believe you're on the right path with LiteralFlag.

Awesome!

DRF returns the representation by default but from python code the enum variant are used unde their "real" name. So I am not sure about how to handle this on Elm side.

Could you add a clear example of this? It's difficult for me to visualize exactly what you mean, but perhaps if there were examples you could show I might be able to help out more.

@YievCkim
Copy link
Author

DRF returns the representation by default but from python code the enum variant are used unde their "real" name. So I am not sure about how to handle this on Elm side.

Could you add a clear example of this? It's difficult for me to visualize exactly what you mean, but perhaps if there were examples you could show I might be able to help out more.

of course exactly here . In my project I have one or several User Stories given to the Elm page by Django but sometime I need to refresh one of them either because the user has just modified it with an action, or because it has been changed by someone else. DomainError is a variant for a specific error my DRF api returns when an operation doesn't match the domain rules (typically an invalid change from an old to a new status).

The status in the UserStory is a Django TextChoice and it is handled directly by DRF .

This is an example of the content DRF provides:

{
    "id": 11,
    "url": "http://127.0.0.1:8000/epics-api/stories/11/",
    "pub_date": "2024-05-10T20:12:16Z",
    "title": "title",
    "description": "bla bla bla.",
    "epic": "http://127.0.0.1:8000/epics-api/epics/1/",
    "assigned_to": null,
    "status": "in progress"
}

As you see status is given with its string representation and not with its python value. This is expected from an api rest point of view, but may not be easy for Djelm to handle if it expects the actual value of the python enum (i.e. StoryStatus.IN_PROGRESS in this case).

@Confidenceman02
Copy link
Owner

Thanks for the link!

It's cool you are building this with DRF, I actually didn't think of this use case! I have always worked with Django as an MPA and used HTMX to make it feel like an SPA.

There are a lot of things that Djelm could do more of to help with this type of application. Nice!

As you see status is given with its string representation and not with its python value. This is expected from an api rest point of view, but may not be easy for Djelm to handle

The DRF returning a string for the TextChoice actually doesn't matter for Djelm or the StringLiteral impementation I am thinking about. The flag declarations will build the right decoder and it will resolve to the InProgress custom type you would expect with the string as a parameter. So all would be ok here! This is the beauty of the Djelm codegen compiler!

If we build a TextChoiceFlag or something like that, Then the python values is a concern only for flag validation, which happens on the server, the Elm decoders and resolvers that are generated will respect the Flag and do the right thing.

@Confidenceman02
Copy link
Owner

String literals released in 0.12.0 should make this far less hacky!

Closing this issue and Tracking TextChoices flag in #103

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

No branches or pull requests

2 participants