Make the State-property on Record more graceful

[LennardF1989]

Pretty much everything is in place to support custom form State values:

• State is stored in the DB as StateAsString.
• Enums can be any number integer value, so var state = (FormState) 99; is a valid C#. So one can make an "extended" enum with custom states.

However, the State-property on the Record-type is ruining the party by using Enum.Parse instead of Enum.TryParse.

There are multiple ways to set the right State and/or StateAsString (Workflow, overriding RecordService), but this one function is breaking it all.

Could you make this method more error tolerant, please 🙈?

[AndyButland]

I'm not sure about this @LennardF1989... it sounds like an innocuous update but I'm not sure we should do it and thus give the impression the product supports custom state values when really we don't. As this would maybe allow you to save them to and load them from the database, but you still wouldn't have access to them via the UI (whether it be for associating workflows, or changing state via the entries screen). There would also be concerns that if we introduced a new state in future, how would that affect anyone who are using custom ones that now clash?

Feels that really this should be a properly supported feature or not at all. That's my initial thoughts anyway... feel free to argue otherwise!

[LennardF1989]

I would consider this an unsupported and undocumented feature if implemented. To properly support custom states, one has to override the original RecordService and implement their own Submit behavior. Which I did, and is not that hard, but the blocker is that nasty Enum.Parse.

As for how to prevent clashes in the future: I think that's on the developer, but I wouldn't put my custom states anywhere earlier than integer value 1000 :P

To test my claim, I've used Harmony to make an in-memory patch:

public enum FormStateExtended
{
    Opened = 0,
    Resumed = 1,
    PartiallySubmitted = 2,
    Submitted = 3,
    Approved = 4,
    Deleted = 5,
    Rejected = 6,

    Test = 1000
}

[HarmonyPatch(typeof(Record))]
[HarmonyPatch(nameof(Record.State), MethodType.Getter)]
public class RecordGetStatePatch
{
    public static bool Prefix(Record __instance, ref FormState __result)
    {
        //__result = (FormState)Enum.Parse(typeof(FormStateExtended), __instance.StateAsString);

        if (Enum.TryParse(typeof(FormStateExtended), __instance.StateAsString, out var value))
        {
            __result = (FormState) value;
        }

        return false;
    }
}

[HarmonyPatch(typeof(Record))]
[HarmonyPatch(nameof(Record.State), MethodType.Setter)]
public class RecordSetStatePatch
{
    public static bool Prefix(Record __instance, FormState value)
    {
        __instance.StateAsString = Enum.GetName(typeof(FormStateExtended), value);

        return false;
    }
}

Whenever I set the Record State to "Test" from (for example) a workflow, everything will load and save accordingly.

By default, the Entries-filter won't include the "Test"-state, but that is easily fixed with an Angular-hook (or by using Harmony to patch the API).

As said, it's hacky, undocumented, but nothing stops one from using custom states. Literally everything is in place.

If anything, the most "hacker friendly" way of supporting this unorthodox use-case is the following:

public static Type FormStateType { get; set; } = typeof(FormState);

[Ignore]
public FormState State
{
  get => (FormState) Enum.Parse(FormStateType, this.StateAsString);
  set => this.StateAsString = Enum.GetName(FormStateType, (object) value);
}

That would allow someone to plugin their own FormState enum, and everything should remain working as long as they support the original states too.

One could argue "If Harmony works, why not just use that", and that person would be totally right! But given the small adjustment that has to be made to make it work pretty much out-of-the-box without having to resort to hacks, made me think it was worth taking a shot by opening this issue :)