Edit Person button in the Person "more info" dialog (fixes #4706)

[spencerwi]

Proposed change

In the "more info" for a Person entity, this PR allows the quick-go-to-edit "pencil" button to be shown, which will navigate to the Person config panel. It also adds an after-data-fetch check to the Person config panel for the URL segment added by that navigation indicating a Person that is intended to be edited. If that URL segment is present, and the Person specified matches one of the Person entities that was fetched, then the page will open the "Edit Person" dialog without requiring additional user intervention.

The resulting user flow is that a user can click on a Person entity anywhere in Lovelace to view the "more info" dialog, then click the "pencil" icon (if they're an administrator), which will navigate them to the People config page and automatically open the "edit person" dialog for that Person entity.

Type of change

• Dependency upgrade
• Bugfix (non-breaking change which fixes an issue)
• New feature (thank you!)
• Breaking change (fix/feature causing existing functionality to break)
• Code quality improvements to existing code or addition of tests

Example configuration

Should be verifiable from the default Lovelace view that shows all entities -- click a Person entity to launch the "more info" dialog.

Additional information

• This PR fixes or closes issue: fixes Edit Person button on more info dialogs #4706
• This PR is related to issue: Edit Person button on more info dialogs #4706
• Link to documentation pull request: (none applicable here, I think?)

Checklist

• The code change is tested and works locally.
• There is no commented out code in this PR.
• Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

• Documentation added/updated for www.home-assistant.io

[homeassistant]

Hi @spencerwi,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

[spencerwi]

I didn't see any relevant component tests to be updated, though I admit I'm not fully familiar with the project structure yet.

[spencerwi]

Are there any component tests that should be updated here? I don't see any that I could use as a "base" for writing any new tests.

[zsarnett]

We dont usually run test in the Frontend. At least not yet. Not sure why that checkbox is there. I think it was from another repo

[spencerwi]

Hey, sorry -- I didn't mean to pester. I just noticed the empty checkbox on the template and wanted to make sure I hadn't missed a step. Thanks for the review!

[bramkragten]

Suggested change

	const isPersonSpecifiedInRoute =
	this.route?.path && this.route.path.includes("/edit/");
	if (isPersonSpecifiedInRoute) {
	const routeSegments = this.route.path.split("/edit/");
	const personId = routeSegments.length > 1 ? routeSegments[1] : null;
	if (personId) {
	const personToEdit = this._storageItems!.find((p) => p.id === personId);
	if (personToEdit) {
	this._openDialog(personToEdit);
	}
	}
	}
	if (!this.route.path.includes("/edit/")) {
	return;
	}
	const routeSegments = this.route.path.split("/edit/");
	const personId = routeSegments[1];
	if (!personId) {
	return;
	}
	const personToEdit = this._storageItems!.find((p) => p.id === personId);
	if (personToEdit) {
	this._openDialog(personToEdit);
	}

[bramkragten]

Try to break out as soon as possible instead of nesting if's

[bramkragten]

And we should show a message when we can't find the person they want to edit

[bramkragten]

The person will always have an ID, also when it is not editable (YAML).

[bramkragten]

There is an attribute, editable: true that we should use.

[spencerwi]

Ah, interesting. I'll update to handle that case.

Would you be alright with it if I extracted this conditional from the HTML template into a getter method so that it's a bit easier to work with?

frontend/src/dialogs/more-info/ha-more-info-dialog.ts

Lines 140 to 143 in 34c2742

	${this.hass.user!.is_admin &&
	((EDITABLE_DOMAINS_WITH_ID.includes(domain) &&
	stateObj.attributes.id) ||
	EDITABLE_DOMAINS.includes(domain))

[bramkragten]

Yeah that's fine, I would not add person to the EDITABLE_DOMAINS_WITH_ID though, as it would need a different check.

[spencerwi]

I think I understand a bit better now -- I understood EDITABLE_DOMAINS_WITH_ID to mean "domains that are editable, and have an ID parameter that should be passed along", but it seems to instead mean "domains that are only editable if an ID is present". I may add a JSDoc comment to that effect, if that's fine with you.