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

Refactor respond_to etc. #39

Open
multiscan opened this issue Jan 29, 2025 · 0 comments
Open

Refactor respond_to etc. #39

multiscan opened this issue Jan 29, 2025 · 0 comments

Comments

@multiscan
Copy link
Contributor

There is a lot of inconsistency in the way we write the controllers. I would like to

  • keep it consistent by deciding once for all how we nest respond_to and if model.save;
  • make sure that the flash message is always shown (both for confirming success and for notifying failure);
  • make sure that we always treat the failures in model creation/update.

Regarding respond_to I think a reasonable way of doing this is not to add empty format.xxx and let the base figure out the available formats by looking into the files unless this adds latency. In other words, when there is nothing to add to the format.xxx like in the following example:

respond_to do |format|
  if model.save
    format.html
    format.turbo_stream
    form.json
  else
    format.html { ... , status: :unprocessable_entity }
    format.turbo_stream { ... }
    format.json { render json: model.errors, status: :unprocessable_entity }
  end
end

I prefer it to be more concise like the following

unless model.save
  respond_to do |format|
    format.html { ... , status: :unprocessable_entity }
    format.turbo_stream { ... }
    format.json { render json: model.errors, status: :unprocessable_entity }
  end
end

The controller will automatically render the appropriate view files in case of success.

It is also important not to forget to send (and update) the flash message in case of success. For turbo_stream format, this involves

  1. add the message in the controller using flash.now:
    flash.now[:success] = "flash.generic.success.create"
  2. refresh the flash notification in the view:
    <%= turbo_stream.replace("flash-messages", partial: "shared/flash") %>
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

1 participant