Is PRG a valid technique in Phoenix?

Phoenix Forum Chat / Discussions
, ,
14

So, getting back on the topic about the PRG pattern… What are the options here (if any)?

15

To me it’s not clear what your remaining issues are - other than perhaps a misaligned mental model which can easily happen when moving between frameworks.

A far as I can tell Post/Redirect/Get is what is happening in

https://media.pragprog.com/titles/phoenix14/code/ecto/listings/rumbl/lib/rumbl_web/controllers/user_controller.change3.ex

in the success case - though perhaps create should redirect to show rather than index.

Then, when you submit your form, the resulting action of the post is always a redirect…

I don’t see any reference of what is supposed to happen in the failure case.

With PRG, we would make the “new” action always responsible for knowing how to build the form.

This is where there is some potential misalignment - it’s the view (new.html.eex) which builds the form - both the new and create functions have the responsibility of providing the necessary data for the view to do its job.

  • the car_colors/car_optionals issue is typically resolved with extracting a common function (likely a context function)
  • alternate solution: cache the encoded data in the server side session so the failure case doesn’t have to hit the database again (but the result could still be in the RDBMS cache anyway).

I guess that if the server is always responding with a 200, it’s still a “valid form submission” for the browser’s point fo view (usually, frameworks don’t return a 500 when there’s a validation problem).

200 OK

The HTTP 200 OK success status response code indicates that the request has succeeded.

The request has succeeded inasfar it was able to reply with a meaningful response - the error page.

If a new resource is created the redirect will cause a 302 Found (ideally it should be 303 See Other (Plug.Conn.put_status/2) while an API should simply use 201 created but that wouldn’t cause a redirect on a browser).

So the fundamental problem doesn’t seem to be the PRG pattern but the fact that you would like the new function to be solely responsible for the “new form” - and Phoenix doesn’t work that way.

16

That’s for sure one of the reasons I brought this topic for discussion.

But regarding the PRG pattern in general, if you check the Wikipedia article, you’ll see that it states that any post request should be followed by a redirect, not only “successful” ones. This, of course, helps in some cases:

Side note: I even saw someone on Stack Overflow suggesting the Strict PRG and Loose PRG terminology - I’m talking mainly about using the former since the latter is already used by default.

  • Avoid form resubmission on page refresh or navigation
  • Bookmarks (keeping resource “state”)
  • Form data building (SRP by extension)

Currently, only the last issue can be solved - by reusing the code (which does not depend on the framework).

In the Stack Overflow thread you linked, there are some assumptions on the “Loose PRG” type mainly because it does not touch more uses-cases for the “Strict PRG” approach, for example…

About the form resubmission issue: as I explained here, it still may be undesirable to allow that behavior, which might turn into a UX problem.

For bookmarks (in the specific case of Phoenix ), there’s that particularity about the routes. By default, if the form contains invalid data, ~/user/create becomes ~/user and ~/user/1/edit becomes ~/user/1 which both are different views (templates) if accessed without the operation context. Then, if the user is presented with an error and I ask him to send me the url where the problem happened, that will be an issue (of course, this is the most simplistic use-case I could think of).

Regarding rebuilding the view, I’ve seen people argue that using the post action to render a view violates REST. I’m not completely sure if this is technically correct, but it’d be good to allow that when possible as a best practice IMO.

I couldn’t agree more (I kept my last paragraph for reference). Thinking that way, I guess you are saying that the view is responsible for rendering the template, so I’m assuming you are also saying that there’s not SRP problem (Could you expand on that?). If so (and that’s also the case), would it still be a REST violation, considering that the action is started by the controller (because the view is used indirectly for the same purpose)?

Not exactly, but I think that this is covered by what I’ve already mentioned.
Also, as you already pointed out before, I’m adapting my mental model by trying to see what could/ should be transferred to Phoenix. So I think is plausible to have these exchanges on how others are approaching it - it’s certainly productive to me, at least.

and Phoenix doesn’t work that way

I opened this thread already thinking that since everything has to be passed explicitly as a param it probably couldn’t be achieved - since the “Strict PRG” presumes that the form-state data has to be sent over on the redirect (or at least kept in some way).

PS.: Was searching for solutions in other frameworks and stumbled upon this rails gem called rails-prg. It seems that there’s another issue that PRG solves that I was not aware of - don’t know if this affects Phoenix too, but thought would be nice to share.

1 Like
17

Professional ASP.NET MVC 2 (2010) attributes that to Phil Haack

Note that redirecting the request only upon success technically violates the PRG pattern, but because no changes where made to the state of anything when validation failed, it doesn’t suffer from most of the problems with not following the PRG pattern. Phil likes to call this loose PRG as opposed to strict PRG.

The pattern itself seems to go back to this 2004 article/2003 discussion:

https://www.theserverside.com/discussions/thread/20936.html

This approach provides a clean Model-View-Controller solution.

Aside: Server MVC has always played it fast and loose with Trygve Reenskaug MVC.

But please dear browser, do not save snapshots of a live program, because they may not represent actual Model state anymore.
…
Now I ask the same question again: what would a user see if he clicks Back button after submitting a form? You know the correct answer already: the user of a well-designed web application would see a View which represents current Model state. This View would be presented in a way that resubmitting of the same data would be impossible.

Sorry, in 2019 browsers still don’t work that way (bfcache, “Backward-Forward Cache”). Even after a successful redirect I can back up to the submit form and those last valid values will reappear in the form fields and I can click that submit button again.

If you need to defeat cached pages you need to employ token matching and remove the hidden form token from the server session after the resource has been successfully created (the standard CSRF token doesn’t stop double submission from a recently cached form).

Browser-side the pageshow event could be used to clear the input fields - but that will only work when JavaScript is enabled.

(1) Create Item is called from a link on some other web page when a new object should be created. This action constructs empty business object and stores it in the temporary area called Current Items, which itself can be stored in the database or in the session; then redirects to Display Item.

This suggests that each user has their own /users/new resource (otherwise it couldn’t be stored in the session) - this violates the notion that /users/new uniquely addresses a single resource.

(2-2) User fills out object value and submits HTML form to the Store Item action. If object is not accepted, it is kept in the Current Items area, server redirects back to Display Item action, which reads invalid object along with error messages from the Current Items and redisplays it in the form. If Item Page needs to be is refreshed, it loads the same object from Current Items again.

i.e. once again - my /users/new would show my last errors while your /users/new would show your last errors - so /users/new isn’t uniquely addressing a resource.

Furthermore we’re now in a situation where errors will change “model state” because the errors are part of the “model state”. So rather than saying that redirect on failure is a requirement, it is more to the point that redirect on state change is a requirement.

If your philosophy is to not accept invalid data then you aren’t going to store “half-baked”, inconsistent items - which is what is happening in the “Current Items” area.

If a user clicks Back button on result page (3) after successfully creating and storing new object, he returns to Display Item action (2).

This completely ignores the existence of the “Backward-Forward Cache” in browsers. (Though it’s presumably preventable with Cache-Control: no-store)

Ultimately the article reads like somebody is trying to coerce their vision of MVC/OO onto the web.

As I explained above with “Current Items Storage” each user/session would have a different /users/new resource anyway - there is no way you could GET /user/new with the validation errors unless you are in the same user/session context.

Phoenix is pragmatic about it - the form validation errors are ephemeral - they aren’t associated with an addressable resource. The aim is to maintain only valid resources, validation errors are ephemeral (short of what is written to the log).

  • when you GET \users you will always get a list of users
  • when you GET \users\new you will always get an empty form

I’ve seen people argue that using the post action to render a view violates REST.

RFC 2616 9.5 POST

The action performed by the POST method might not result in a resource that can be identified by a URI. In this case, either 200 (OK) or 204 (No Content) is the appropriate response status, depending on whether or not the response includes an entity that describes the result.

So 200 OK for the validation errors is permissible as no resource is created.

If a resource has been created on the origin server, the response SHOULD be 201 (Created) and contain an entity which describes the status of the request and refers to the new resource, and a Location header (see section 14.30).

This is the ideal REST case - typically used by APIs.

However, the 303 (See Other) response can be used to direct the user agent to retrieve a cacheable resource.

This is what is used to guide browsers to the newly created resource.

FYI: SRP/SOLID

“Gather together those things that change for the same reason, and separate those things that change for different reasons.”

  • new.html combines for user convenience entry fields with “validation error placeholders”.
  • UserController.new uses new.html strictly for the entry fields.
  • UserController.create uses new.html for both the entry values and validation errors.
  • Separate new.html and create_error.html views would duplicate markup and functionality. It makes sense to combine them into one new.html, especially as they are essentially coupled anyway.

If so (and that’s also the case), would it still be a REST violation, considering that the action is started by the controller (because the view is used indirectly for the same purpose)?

  • new.html is the representation for GET /users/new (i.e. it’s an empty form)
  • index.html is the representation for GET /users
  • in the case of create (POST /users) new.html is “reused” to convey the error information - i.e .it is not a representation of any particular resource.

I opened this thread already thinking that since everything has to be passed explicitly as a param it probably couldn’t be achieved.

It’s just not something that is covered out-of-the-box.

You are free to implement your own “Current Items” scheme - though in my personal view it’s an un-webby thing to do.

1 Like
18

@peerreynders Although I don’t think that my main concerns were addressed so far, there’s certainly a lot of useful information in this thread…

Since the reason I’ve posted was basically to see how this is being solved on the Phoenix side, I’m gonna be a little bit more practical and reformulate the question because I think if we keep expanding on other definitions we are not going to reach a conclusion.

By default, if the user is filling a form on ~/user/new and there are errors on the form, because of how the routing works, the url changes to ~/user. I see two issues here that I want to be able to solve and using PRG helps me:

  1. If the user clicks the refresh button, it shows the resubmission message which may not be desirable for UX purposes.
    The user might expect the page to be rebuilt instead (which is also a question posed in the “Redirect After Post” discussion and was not addressed).
    - There’s also the suggestion of using tokens, which does not solve the problem for other complex scenarios as described in the same discussion.

  2. If the user renavigates to the current url after posting (selecting the search bar and pressing enter for example), it’ll load the ~/user url, which shows the index page and not the form page. Then, the only way the user can start fresh in the form is pressing the backwards button - which will navigate to the previous ~/users/new route.
    - This can also be counter-intuitive for mobile browsers where the forward/ backward buttons are hidden. Also, I’ve seen that some users tend not to use this feature and instead always reload the page. This combined with the fact that some devices (I guess mainly Android) have inconsistent backwards navigation, it’s a huge UX problem.
    - Beyond that, the problem of ‘bookmarkability’ that I think was misunderstood is not about preserving/ sharing error state, is about correctly identifying the resource (form). So in an error scenario, the user will be presented with the /users url instead of the right url of the page he’s at (~/user/new).

  3. I want to separate responsibilities of building the form and actually creating it (but you pointed out that Phoenix does not work like that before. So I wonder if there are any other options).

It’s just not something that is covered out-of-the-box.

I’m completely fine with that, aspnet also does not support PRG by default but there’s at least a way to transfer data on redirects (Laravel also does that).

Do you think that the concept of a session temp-data that is short-lived in between redirects would break the Phoenix paradigm of doing things? Right now, I’m not sure how something similar could be achieved.

in the case of create (POST /users) new.html is “reused” to convey the error information - i.e .it is not a representation of any particular resource

But the route belongs to another resource. As I pointed above this can cause confusion.

“Gather together those things that change for the same reason, and separate those things that change for different reasons.”

I’m familiar with this definition. But it seems that this “change for the same reason” is sometimes drawn arbitrarily. So, for example: UserController.new beeing only responsible for building and displaying the form (with and without errors) and UserController.create only beeing responsible for actually posting data and redirecting is also a valid interpretation. I find difficult to argue that this does not make sense, also from the resource point of view of REST where “GET only reads and POST only writes”, but that’s not the point.

19

I just want to add that when you don’t want the default urls generated by the resources/4 macro, you could always use directly get/4 and post/4 like below:

# Registration
  scope "/registration", DemoWeb do
    pipe_through :browser
    get "/", RegistrationController, :new
    post "/", RegistrationController, :create
  end

This way the url won’t change between empty new form and create_error form. In my case I just do that for SEO purpose (for example I don’t want the url in English). And if someone shared the url, clicking on it will always result on the GET new form.

If you still want to redirect on invalid submit, you could assign the invalid changeset to the conn passed to redirect/2. Then you could have two clauses for your new action, one for empty form and the other for invalid form:

 # invalid form
  def new(%{error_changeset: changeset} = conn, _params) do
    render(conn, "new.html", changeset: changeset)
  end

  # empty form 
  def new(conn, _params) do
    changeset = Context.change_resource()
    render(conn, "new.html", changeset: changeset)
  end
2 Likes
20

Wow! That’s a very simple solution, thanks for that :blush:

If the conn.error_changeset is kept intact on a redirect, that means that I could also just assign the changeset variable conditionally whether or not I have errors coming through - is that right?
I guess this solves the problem of sending the form state over and allows me to do a full PRG.

21

Yes, you can get it with something like:

changeset = conn.assigns[:error_changeset] || Context.change_resource()

So in my previous answer the matching clause would rather be

%{assigns: %{error_changeset: changeset}} = conn

If you usePlug.Conn.assign/3 to assign the changeset it will be available in conn.assigns.
Using a dot to access a key not present in a map will throw an error that’s why it’s better to use [:key] which will return nil if not found.

1 Like
22

@Kurisu Do you have a working example?
I made some tests and the errors are not showing after the redirect for some reason.

# new
changeset = conn.assigns[:errors] || Context.change_resource(%Resource{})
render(conn, "new.html", changeset: changeset)

# update
conn = %{conn | assigns: Map.merge(conn.assigns, %{:errors => changeset})}
redirect(conn, to: Routes.resource_path(conn, :new))

Update: Seems that the assigns are been cleaned after the redirect. I think that this is not possible without using session.

1 Like
23 
redirect(conn, to: Routes.resource_path(conn, :new))

sends the POST response back to the client.

The followup GET from the client is an entirely different request and therefore has a new conn with a fresh assign - nothing is getting “cleaned”.

You have to bridge the information between the two separate requests.

  • Using the redirect the invaild form data could be transferred as query parameters in the redirect URL - though that’s hardly desirable.
  • The create function could stick the invalid form data in the server side session to be retrieved by the new function to be used to initialize the changeset.
1 Like
24

Yes of course. I fooled myself thinking that maybe Phoenix could be doing some “magic” behind the scenes. But it makes total sense that the “conn” struct is different because it’s a different request.

I’m gonna study other options of what can be done, but I think this could be encapsulated in a helper? Something like a redirect_with/3, to put the errors on the session and then retrieve on the other side (maybe a plug?).

Update: So, I’ve read a little bit about Plug and this is what I came up with (It’s probably a naive implementation, but right now I can get full Post Redirect Get):

defmodule MyAppWeb.Plug.TempData do
  import Plug.Conn
  use MyAppWeb, :controller

  @session_key "temp_data"
  @assigns_key :loaded

  def init(opts), do: opts

  def call(%Plug.Conn{private: %{:plug_session => %{@session_key => temp}}} = conn, _opts) do
    conn
    |> assign(@assigns_key, temp)
    |> delete_session(@session_key)
  end

  def call(conn, _opts), do: conn

  def load_redirected(conn, default) do
    conn.assigns[@assigns_key] || default
  end

  def redirect_with(conn, object, opts) do
    conn
    |> fetch_session()
    |> put_session(@session_key, object)
    |> redirect(opts)
  end
end

Then it can be used like:

import MyAppWeb.Plug.TempData, only: [redirect_with: 3, load_redirected: 2]

alias MyAppWeb.Plug.TempData

plug TempData

def new(conn, _params) do
    changeset = load_redirected(conn, Context.change_resource(%Resource{}))
    render(conn, "new.html", changeset: changeset)
end

def create(conn, %{"resource" => resource_params}) do
    case Context.create_resource(resource_params) do
      {:ok, resource} ->
        conn
        |> put_flash(:info, "Resource created successfully.")
        |> redirect(to: Routes.resource_path(conn, :show, resource))

      {:error, %Ecto.Changeset{} = changeset} ->
        redirect_with(conn, changeset, to: Routes.resource_path(conn, :new))
    end
  end
1 Like
25

I thought it would be a good idea to extract this little functionality in its own specific package.
So, here it is: briefcase - It’s just a simple plug to scratch this little itch. If anybody is searching for a similar solution, feel free to take a look and to contribute :relaxed:

PS.: Since I’m not yet well versed in everything elixir has to offer, any suggestions are extremely welcome.

1 Like
26

Hello @thiagomajesk Not sure if you are still active.

I encounter the same sort of situation where I need to apply a redirect for the submission of an invalid changeset. I think I have a valid scenario. I will explain why:

  1. The user lands on the page displaying the form, allowing him to add other users (GET)

  2. The user submits the form (POST)

[the form is valid] ->
⠀⠀⠀3. A redirect is done to the next page (GET)

[the form is invalid]
⠀⠀⠀3. The page is re-rendered showing the errors from the invalid changeset (same request)
⠀⠀⠀-> Back to point 2.

  1. The user presses the ‘Back’ button of the browser.

If he submitted a valid form immediately, when clicking the ‘Back’ button the user will go back to point 1., and a GET is executed, which will not lead to any errors, because the controller action is called and reloads all the data. The flow:
[GET display form page] (A) -> [POST submit form] (B) -> [GET redirect] (.C) -> [GET pressed back button] (back to A)

If he submitted an invalid form, corrected it, and then submits again the flow is:
[GET display form page] (A) -> [POST submit form] (B) -> [POST submit form] (.C) -> [GET redirect] (D) -> [POST pressed back button] (back to B, which is POST)

The latter will lead to an error, when he entered a new user (POST), applied a correction in the form (second POST), and then it got inserted into the DB; then presses back => the form will not include the hidden input containing the ID of the user. So submitting again will cause an error where Ecto cannot find the user that is already saved in DB in the form data.

So I noticed this is how browsers behave when hitting ‘Back’: when the ‘Back’ action returns to a GET request, the server is actually called, and so the controller action will load all the correct data. However, when the ‘Back’ action returns to a POST request (and this happens for a form that must be corrected and leads to multiple subsequent POST requests), nothing is executed on the server; the browser loads the page from some cache. And so, the form will show incorrect data (more specifically here, lacking hidden inputs containing IDs for entities that have been already inserted).

Anyway, besides your quite complex solution (I imagine it is complex as even in your readme of that lib you made, you said it’s only experimental), what would be the best solution? Are there any other trivial solutions (even if it doesn’t provide a smooth UX) to overcome this problem?
Most important is to not raise an error and show an error page to the user, as my form behaves now in such specific case.

27

@thojanssens1 Hello!

If I understood correctly this can be a perfect use case for using a post-redirect-get approach.
One of the main advantages is again, separating the responsibilities and keeping proper page state. I’m positive this is not always the case for a lot of people and is mostly useful for server-rendered page apps.

I think you missed a step there because when failures happen on PRG, you should redirect back to the page that knows how to build the form so the user can correct it and submit it again as a clean slate.
So, from the client perspective:

  1. [GET] Renders user’s form page
  2. [POST] Submit (yields validation error on the server-side)
    This should redirect back with invalid form state
  3. [GET] Renders user’s form page with validation messages

The lib I’ve created goes into step 2, saving the changeset yielded and passing it to the action that builds the form so it can display the errors properly. Right now, because of the redirect-back, the browser history gets a little bit messy if the user keeps entering invalid data (since the page is always refreshed back).

Actually, the code itself is not complex. At its core, it’s only a Plug that adds and retrieves data from the session. It’s experimental so far because I’m not using it on production and I consider it mostly a Proof of Concept, but if it fits your needs just go ahead and use it.

28

Oops I wasn’t clear. I described the behavior with a default Phoenix setup, i.e. redirect on successful submit, and no redirection if errors in changeset. Not full PRG. To justify why I need in my case to redirect even when there are errors in the changesets.

I had a quick look at the library’s code and, there’s quite some:) I was wondering, why didn’t you use the Flash messages functions get_flash/put_flash, as it seems your library uses the same mechanism?

29

The main difference is in the lifecycle. Since Briefcase does not rely on the connection it can go back and forth as long as you have clean values (not dirty) saved in the internal store (session).
If you want to know exactly why there’s this previous response that explains (I too didn’t think of it first but it’s actually pretty obvious once someone has pointed out :sweat_smile:) .

30

The response you linked, says you have to send data between two different requests. So indeed, it’s quite obvious that you can’t put it in the conn’s assigns, as that data will be lost after redirection.

But this is exactly what the flash message is for, i.e. store temporarily data in session for persisting data in between two requests; and flash messages should also be marked “dirty” once they have been retrieved.

31

I’m not 100% sure about this but I think is just the Phoenix request lifecycle that makes the flash messages end up in the session…

The underlying %Plug.Conn.put_private/3 that is used by Phoenix.Controller.put_flash/3 only places the values inside the conn.private map. I’m thinking that Phoenix.Controller.fetch_flash/2 in the :browser pipeline is actually what makes the magic happen for us:

router.ex

pipeline :browser do
    plug :accepts, ["html"]
    plug :fetch_session
    plug :fetch_flash
    plug :protect_from_forgery
    plug :put_secure_browser_headers
end

controller.ex

cond do
    is_nil(session_flash) and flash_size == 0 ->
        conn
    flash_size > 0 and conn.status in 300..308 ->
        put_session(conn, "phoenix_flash", flash)
    true ->
        delete_session(conn, "phoenix_flash")
end

Moreover (If I’m correct), because Briefcase does not depend on Phoenix we can’t rely on this lifecycle.
I guess we could actually persist our information inside conn.private as a convention and then place it on the session, but right now we just use the session directly.

32

The session must be used sparingly. Getting the data from conn.private and putting it into session before the response is sent is specific to the built-in Flash functionality. It’s not part of the general request lifecycle; it would be terrible if it were.

fetch_flash/2 returns a register_before_send/2 function, which is a callback that gets called before the response is sent. And from there the data from conn.private is stored in session.

After some research, a simple solution using the built-in Flash mechanism to implement full-PRG (so including redirecting after invalid form submission), are the following steps:

  1. Invalid form is submitted, create_* action is called
  2. Submitted form parameters are put into Flash, if changeset is invalid
  3. Redirect to the new_* action
  4. Rebuild the changeset based on parameters that were stored in Flash (if present)
  5. Form displays all errors

Sample code (example is a form allowing to add/edit/remove items for a user):

def new_user_items(conn, %{"user_id" => user_id}) do
  changeset =
    Business.get_user!(user_id, preload: [:items])
    |> Business.change_user(get_flash(conn, :create_user_items_params) || %{})  #  <---
    |> Map.put(:action, :update)

  render(conn, "new_user_items.html", changeset: changeset, user_id: user_id)
end

def create_user_items(conn, %{"user_id" => user_id, "user" => user_params}) do
  user = Business.get_user!(user_id)

  case Business.add_items(user, user_params) do
    {:ok, _} ->
      redirect(conn, to: Routes.registration_path(conn, :next_action, user_id))
    {:error, %Ecto.Changeset{} = changeset} ->
      conn
      |> put_flash(:create_user_items_params, user_params)  #  <---
      |> redirect(to: Routes.registration_path(conn, :new_user_items, user_id))  #  <---
  end
end
33

Maybe I was not clear enough: I didn’t mean it’s part of Plug’s request lifecycle - It’s actually part of Phoenix’s browser pipeline, but I’m not going to get cought in discussing semantics if I can avoid it.
In my experience whether you should or shouldn’t use the session has to be decided in a case-by-case basis.

Maybe I should have been a little more specific, but If you see my previous comment I specifically linked to the code inside the register_before_send function, since that section is only called because fetch_flash is include in the pipeline :sweat_smile:.

About using just put_flash: though, I wouldn’t go this route to write a lib (since this behaviour doesn’t exist for Plugs) I guess it’s perfectly fine if you are using Phoenix and want a “quick and dirty” solution just to pass data from one request to another.
Also my purpose with Briefcase is to do a little bit more out-of-the-box. I hope this clears things up :slightly_smiling_face:.