core icon indicating copy to clipboard operation
core copied to clipboard

Fix capitalization and grammar in `simplefin`

Open NoRi2909 opened this issue 7 months ago • 3 comments

Proposed change

  • consistently sentence-case "claim token" and "access URL" Also replace one inconsistent use of "Account URL" with "access URL".
  • remove duplicated verbs "used claimed"
  • capitalize "URL" for consistency

Type of change

  • [ ] Dependency upgrade
  • [x] Bugfix (non-breaking change which fixes an issue)
  • [ ] New integration (thank you!)
  • [ ] New feature (which adds functionality to an existing integration)
  • [ ] Deprecation (breaking change to happen in the future)
  • [ ] Breaking change (fix/feature causing existing functionality to break)
  • [ ] 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:
  • Link to developer documentation pull request:
  • Link to frontend pull request:

Checklist

  • [ ] 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] I have followed the perfect PR recommendations
  • [ ] The code has been formatted using Ruff (ruff format homeassistant tests)
  • [ ] Tests have been added to verify that the new code works.

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

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

  • [ ] 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.

To help with the load of incoming pull requests:

NoRi2909 avatar May 05 '25 08:05 NoRi2909

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

Code owner commands

Code owners of simplefin can trigger bot actions by commenting:

  • @home-assistant close Closes the pull request.
  • @home-assistant rename Awesome new title Renames the pull request.
  • @home-assistant reopen Reopen the pull request.
  • @home-assistant unassign simplefin Removes the current integration label and assignees on the pull request, add the integration domain after the command.
  • @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
  • @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

home-assistant[bot] avatar May 05 '25 08:05 home-assistant[bot]

This generally looks fine. However, based on wording on SimpleFINs site, maybe some of our wording should be changed to align.

I went through the flow on my SimpleFIN dashboard to generate a token and I didn't actually see the words "Claim Token" or "Access URL" anywhere. The current flow from the dashboard is to click on "New connection" under "Apps", give it a name, then it'll give you what it calls a "Setup Token". I think there may be value in keeping "Setup Token" capitalized since this is how SimpleFIN capitalizes it and I think identifies it formally.

Might be a good idea to update this in the docs as well.

ScottG489 avatar May 05 '25 14:05 ScottG489

Thank you very much for checking and comparing the current UI! I only had a quick look at this (very outdated) website: https://www.simplefin.org/protocol.html which still talks about the "Access URL".

Then I will replace all occurrences of "claim token" with "setup token" as a first step so this matches again.

I think we should stick to keeping that sentence-cased (as recommenced in Home Assistant) as "setup token" can be translated as it is not a name but a normal word. For English users this will still match 1:1.

Please have a look at the error messages talking about the "access URL" and what we should do about those – probably in code, too.

NoRi2909 avatar May 05 '25 17:05 NoRi2909

Haha yeah, it's a great service but even the documentation is pretty "simple" :)

I'll take a look at the error message. FYI there's also this documentation: https://beta-bridge.simplefin.org/info/developers

A lot of the terms are mixed up as well. Like what it refers to as an access token is actually a URL (as you can even see in the example). I figured to keep things as simple as possible we'd just reference the setup token. All end users really need to know about is the setup token and they can just find that through the bridge UI app setup process. They don't need to read any dev docs.

ScottG489 avatar May 06 '25 14:05 ScottG489

OK, so as the HA integration only needs the setup token from the user I removed "or access URL" from the setup process. Now it's just "Please enter a SimpleFIN setup token." and "Setup token".

The error messages can stay as they are right now, that should not be a major issue.

NoRi2909 avatar May 06 '25 18:05 NoRi2909

I think that makes sense.

I reviewed some convos with jeef and the original PR and the reasoning for accepting both was apparently simplicity, but I think allowing more options is just more confusing. It's cool that we accept both, but there's really no downside to creating more app connections via the SimpleFIN dashboard which is how you get the setup token.

ScottG489 avatar May 06 '25 23:05 ScottG489

All is good with me.

jeeftor avatar May 06 '25 23:05 jeeftor

@ScottG489 If you are fine with this PR please approve it

edenhaus avatar May 07 '25 11:05 edenhaus