Add config flow to Jewish Calendar

[yuvalabou]

Breaking change

Add Jewish calendar config flow, one might need to reconfigure the intergartion.

Proposed change

Add Config flow to Jewish Calendar This will also fix the bug with UID changing when location has change.

Type of change

• [ ] Dependency upgrade
• [ ] Bugfix (non-breaking change which fixes an issue)
• [ ] New integration (thank you!)
• [x] New feature (which adds functionality to an existing integration)
• [ ] Deprecation (breaking change to happen in the future)
• [x] Breaking change (fix/feature causing existing functionality to break)
• [x] Code quality improvements to existing code or addition of tests

Additional information

• This PR fixes or closes issue: fixes #
• This PR is related to issue:
• Link to documentation pull request:

Checklist

• [x] The code change is tested and works locally.
• [ ] Local tests pass. Your PR cannot be merged unless tests pass
• [x] There is no commented out code in this PR.
• [x] I have followed the development checklist
• [x] The code has been formatted using Black (black --fast homeassistant tests)
• [ ] 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

If the code communicates with devices, web services, or third-party tools:

• [x] The manifest file has all fields filled out correctly.
  Updated and included derived files by running: python3 -m script.hassfest.
• [ ] New or updated dependencies have been added to requirements_all.txt.
  Updated by running python3 -m script.gen_requirements_all.
• [ ] For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
• [ ] Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

• [ ] I have reviewed two other open pull requests in this repository.

[home-assistant[bot]]

Hey there @tsvi, mind taking a look at this pull request as it has been labeled with an integration (jewish_calendar) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of jewish_calendar can trigger bot actions by commenting:

• @home-assistant close Closes the issue.
• @home-assistant rename Awesome new title Change the title of the issue.
• @home-assistant reopen Reopen the issue.
• @home-assistant unassign jewish_calendar Removes the current integration label and assignees on the issue, add the integration domain after the command.

[yuvalabou]

@thecode Shavua Tov! Please review this one?

[thecode]

@yuvalabou please address the CI failures & the other comments. Can you limit the PR to the minimum required to transition to config flow?

[disforw]

I am using this PR as a custom_componant, and it is working perfectly. Thank you!

[home-assistant[bot]]

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks :+1:

Learn more about our pull request process.

[github-actions[bot]]

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. Thank you for your contributions.

[disforw]

That's too bad, this code works! I'm using it. Is there any way I can take one the PR? Or work it WITH you?

[tsvi]

We need to: [ ] rebase on existing code [ ] fix failing tests [ ] review requested change [ ] change from draft to actual PR

[yuvalabou]

I hope someone else contribute to this PR as I had faced my limits in programming skills

[disforw]

Would you mind adding myself and @tsvi as collaborators to your fork so we can help out on this PR?

[yuvalabou]

I will, however in the meantime you can PR

[github-actions[bot]]

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. If you are the author of this PR, please leave a comment if you want to keep it open. Also, please rebase your PR onto the latest dev branch to ensure that it's up to date with the latest changes. Thank you for your contribution!

[yuvalabou]

Not stale yet..

[jeeftor]

What still needs to be done? I might be willing to take a look

[jeeftor]

Ah -> you are failing tests...

In your dev environment run the following command:

python3 -m pytest ./tests/components/<INTEGRATION_NAME>/ --cov=homeassistant.components.<INTEGRATION_NAME> --cov-report term-missing --durations-min=1 --durations=0 --numprocesses=auto

This might help you fix the tests

aka like so:

python3 -m pytest ./tests/components/intellifire/ --cov=homeassistant.components.intellifire --cov-report term-missing --durations-min=1 --durations=0 --numprocesses=auto

[jeeftor]

Pytest is still failing unfortunately

[tsvi]

All tests pass now. We are ready for a more in-depth review now. We should:

• Confirm that moving from old to new config flow everything works correctly
• Confirm that the behavior still matches the wanted behavior
• With the amount of things that changed, during this development we might have unnecessary extra code, or better way of implementing stuff
• We should document anything that will break

[tsvi]

@yuvalabou Please use git rebase and not merge. See here: https://developers.home-assistant.io/docs/development_catching_up/

[yuvalabou]

My bad, used the "sync repo" button on github on my fork.

[yuvalabou]

encountered a warning in my last try 2024-04-06 18:04:33.235 WARNING (MainThread) [homeassistant.util.loop] Detected blocking call to import_module inside the event loop by integration 'jewish_calendar' at homeassistant/components/jewish_calendar/config_flow.py, line 110: options=sorted(zoneinfo.available_timezones()), (offender: /usr/local/lib/python3.12/importlib/resources/_common.py, line 82: return importlib.import_module(cand)), please create a bug report at https://github.com/home-assistant/core/issues?q=is%3Aopen+is%3Aissue+label%3A%22integration%3A+jewish_calendar%22

[tsvi]

@yuvalabou maybe we should remove the name at all. I think we should only have a single entry for the Jewish calendar integration (like the sun integration).

[disforw]

I can confirm this works in the wild 👍🏼

[tsvi]

I think I took care of all of the suggested changes (except for the unique_id/entity_name which I think should be addressed here as well). Is there a way for @yuvalabou to assign this PR to me in case more work needs to be done? Also @joostlek , @emontnemery and @epenet would like your follow-up after these latest changes.

[joostlek]

You can open PRs to his branch, he could give you access to push on his branch or you would need to open your own PR to continue, but then we should make sure that we don't have a lot of PRs open for the same feature

[tsvi]

I reverted the changes to the unique id and the naming, we'll do those in a subsequent PR. @yuvalabou, please remove from the PR description the fix for the unique id.

[yuvalabou]

Done

[tsvi]

Please add tests to cover all the new code:

* The option flow is not covered by tests

* Add a basic test which shows import flow is triggered

* Add tests which cover the different way in which a config entry can be configured

@emontnemery Could you check please? I think this has been addressed.

[tsvi]

Thank you everyone for your patience and help. This has been a long time in the making, starting with #28289, after which @yuvalabou tried this again in #71166. Thanks especially to @joostlek @emontnemery @raman325 and @epenet for your helpful comments.

[joostlek]

Euh, having a quick look, I don't see any mechanism that starts an import flow and creates an issue @emontnemery

[joostlek]

Oh wait, it has been added in __init__.py. but why is async_setup_platform added in sensor.py?