Properly place fields into entity display on install

I agree, this could be done better. Here is a potential solution: add the field_last_password_reset and field_password_expiration as base fields on the user entity via hook_entity_base_field_info().

@mroycroft yes that would make sense, especially as those fields don't really make sense to remain if password_policy is uninstalled (see #2771129: "Field field_last_password_reset is unknown" while importing configuration on site without password_policy activated- though I suspect a hook_uninstall would still be needed to remove the fields on uninstallation even if they are originally added from a hook_entity_base_field_info() implementation).

I am obviously not providing the entire solution here but I think that this kinda gets more important as it is blocking proper profile setup.

I have a case where the default user display form is installed in the profile. The problem is that the profile does not override existing configuration if it already exists. So what happens, is that password_policy which is a requirement to my profile, installs the user.user.default entity form display and then I cannot override it with my profile. This causes my tests to fail and my user fields to change.
This is certainly not a desired behavior.

I am providing a patch to remove this field setup. I do not know if this is currently a proper solution (is it breaking somewhere I could not notice if I remove this piece of code?). Anyway, in the hook_install, the module is setting a safe default value for the fields (like "do not reset the password of the users") so the fields are not actually necessary and should be up to the user to define them.
The way it is now, the user needs to create a separate module that has this as a dependency and manually override or delete the configuration provided here in order for the profile to work properly (not really convenient).

I also think that providing the fields via hook_entity_base_field_info() is also the proper solution and then do not do anything further than that.
Edit: Actually I do not have a strong opinion there as it is still working without issues for me with the config files. My thoughts are if we generally need to ever set the fields in the form display. We could also set the fields only if the default form display already exists. That would force it to be skipped if it is installed by a profile (in that case the profile should be responsible) and would only be invoked if there is an active configuration. But that would also raise the question of whether the user would agree for fields to randomly appear in his edit screen.

I also tried a clean site install with the patched version and it is working properly.
I checked the https://www.drupal.org/node/2605086 and the module was updated from updating the entity form display as a simple config to updating the entity form display as an entity form display object because it was breaking the /user/*/edit. Though I think that removing it entirely is a more proper solution here.

Also, I propose to change the title to "Do not alter the User's default form display on install" or something similar if the above patch is accepted.

An attempt to update the tests cause my phpunit decided to be stubborn.

Makes sense.

Tested.

Rerolled.

Thank you for the patch but I don't understand it. When you install, the "password expiration" and "last password reset" fields are no longer shown on the user form. You have to install Field UI and then manually enable the field. It seems to me like we want the fields to be added to the display by default, no?

Anyway, in the hook_install, the module is setting a safe default value for the fields (like "do not reset the password of the users") so the fields are not actually necessary and should be up to the user to define them.

My understanding is the primary purpose of these fields is:
(1) To allow an administrator to set whether a user's password is expired.
(2) To allow the user to see when their password was last reset.

In my (cursory) opinion it would be good for these fields to be shown by default. Otherwise the user has no way to see when their password was last reset. It is also a common need of administrators to expire passwords.

I'm sure Password Policy isn't the only module that adds fields to the user form on install.

I suspect the approach from #2771129-86: "Field field_last_password_reset is unknown" while importing configuration on site without password_policy activated that uses base fields would be better than this anyway? There may be more work needed, but at the least, I don't think we need both tickets open.