ryancramerdesign / ProcessWire

Our repository has moved to https://github.com/processwire – please head there for the latest version.
https://processwire.com
Other
727 stars 201 forks source link

form field values after empty page name #523

Open pine3ree opened 10 years ago

pine3ree commented 10 years ago

Hello Ryan,

i have this issues (2.4.0, 2.4.4, 2.4.5).

1. When i edit an existing page, and modify various fields like summary or body, if i accidentally set an empty name and try to save, all the new values are lost and the form is filled with old db values, so that all the work is lost.

2. If i set an empty title the form submission results in validation failure but the page is saved and has an empty title (visited page tree just after validation failure)

3. in multilang installation a required title does force to input a title in other languages.

Can You verify?

kind regards,

maks

ryancramerdesign commented 10 years ago
  1. Name is required on all pages and that's why it's the first thing it asks you to add before saving a page. If you attempt to save a page with no name (in default language), that's a fatal error – there's no way for the save to proceed. That's why your changes weren't saved. It's a bit unusual to have a name field go empty, even if accidental–how is that occurring from the UI standpoint? I suppose we could have it restore the previous name if it came up empty, to prevent a fatal error, but just wondering how common of an occurrence this is (this hasn't come up before, that I'm aware of).
  2. A page can still be saved with an empty title. If title is a required field in your system, then it won't let you publish an unpublished page without a title. But it's not going to prevent saving a published page, it's just going to give you a warning so you know that there's an issue. This is true of all required fields in ProcessWire. The only exception is that "name" field, which is part of the database index and thus a Page can't exist at all with an empty name field.
  3. In a multilang install, a required title would only be required for the default language. Other languages fall-back to the default language value when not present, so only the default language is technically required.
pine3ree commented 10 years ago

Hello Ryan,

thanks for replying.

About 1.

I haven't looked into the saving process yet, i hope i'll have time to get there soon. I have customers who are used to have the page url-segment, slug (the page name in pw) autogenerated from another field (pagetitle in pw) if empty.

If the slug is not unique then i automatically add the table row primary key part as in product-123, 2014-06-21-post-456

So when they change the title they are used to empty the slug input field and let the system do the dirty work for them. In cases like this if many important changes have been made, they would get lost on form submission. In my opinion new user input values should never be disregarded, but presented again along with eventual form errors.

In pw case as well, it think it would be safer to present the new populated values instead of getting the old ones from the db, and just instruct the user where is the error that prevented the page to be saved. (empty name in this peculiar example)

In my long experience with cms and customers, i always saw the least probable events to happen frequently. It's like the cms users are there to mine the rules of probability laws. :)

So in my opinion we should assume that customer will empty the name field (because the cmf allows them to do that and because they have rightfully permission to modify it as they wish) and just prevent any input data loss.

This case could be handled in two ways:

a. presenting an error and instructing the user to fix it b. automatic repopulating the name field generated from the first non-empty pagetitle(language) or with a custom name (maybe based on the template name + page id just to) and then saving the page

kind regards, maks

pine3ree commented 10 years ago

Ok, i've taken a look.

I think that can be accomplished in ProcessPageEdit::processSave() moving: $this->processSaveRedirect($this->redirectUrl); at the end of the try block.

Can you verify that?

kind regards,

maks

ryancramerdesign commented 10 years ago

I don't think it's ideal to remove the redirect or change the behavior of it, just because some things may be depending in it. I'm only in mobile now so can't look close, but am thinking a simple answer would be for processInput to just check if the name is empty and replace it with $page->namePrevious if so, along with an error message.

On Saturday, June 21, 2014, maks feltrin notifications@github.com wrote:

Ok, i've taken a look.

I think that can be accomplished in ProcessPageEdit moving: $this->processSaveRedirect($this->redirectUrl); at the end of the try block.

Can you verify that?

kind regards,

maks

— Reply to this email directly or view it on GitHub https://github.com/ryancramerdesign/ProcessWire/issues/523#issuecomment-46754829 .

ryancramerdesign commented 10 years ago

I don't think it's ideal to remove the redirect or change the behavior of it, just because some things may be depending in it. I'm only in mobile now so can't look close, but am thinking a simple answer would be for processInput to just check if the name is empty and replace it with $page->namePrevious if so, along with an error message.

On Saturday, June 21, 2014, maks feltrin <notifications@github.com javascript:_e(%7B%7D,'cvml','notifications@github.com');> wrote:

Ok, i've taken a look.

I think that can be accomplished in ProcessPageEdit moving: $this->processSaveRedirect($this->redirectUrl); at the end of the try block.

Can you verify that?

kind regards,

maks

— Reply to this email directly or view it on GitHub https://github.com/ryancramerdesign/ProcessWire/issues/523#issuecomment-46754829 .

pine3ree commented 10 years ago

Sure, of course you verify when you have time. I'm just adding now some thoughts before i forget them :-)

I believe that a redirect without a page->save() in pw will always reload old db values, unless we store the new values in the session and repopulate the form (or the $page entity from which the form inputs are built on) with them. (redirecting will call ___buildForm and then $page->getPageInputfields(...) which assigns the $page db values to the fields)

I think that "redirect on success / represent the form with request data on failure" is a common crud pattern, but of course being quite new to pw i am not fully aware of the implications here.

kind regards