Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can't specify a family for family_descend_chart #334

Closed
jeffasuk opened this issue Feb 4, 2023 · 9 comments
Closed

Can't specify a family for family_descend_chart #334

jeffasuk opened this issue Feb 4, 2023 · 9 comments

Comments

@jeffasuk
Copy link
Contributor

jeffasuk commented Feb 4, 2023

(If this looks familiar, it may be because I previously posted this on Gramps discourse.)

I have been trying to generate a “family_descend_chart” report.
I had trouble specifying which family to report on.
I tried: “pid”: “F0098” and got ERROR 422: UNPROCESSABLE ENTITY.
Looking into the code (reports.py), I found that option values are checked for validity against the command’s help text. In the case of “pid” (which probably ought to be “fid”; copy/paste error?), each of the valid values identifies a specific family as ID colon tab family-name; e.g. “F0137:\tDoe, John, Smith, Jane”
Having tried using that as the “pid” value (which didn’t work), I looked further and found that the help values are split at the tab, with only the part before the tab being used.
So I tried “F0098:” that is with a colon after the ID.
That successfully passed validation, but failed later on because, of course, there was no family with an ID of “F0098:”.

I tried changing the split on tab to a split on “:\t”, but of course that broke other options which don’t have a colon.
I worked round this by adding "rstrip(‘:’) to the split-on-tab result. That worked!

A simpler way would be not to use a colon in the help text, but I haven’t been able to find where that text is generated.

If you want to use the rstrip method, I've attached a patch file (against #312). Note that this also removes the unnecessary test for "\t" in item; if there's no tab, split will return a single element list with the original string in it.

reports.TXT

@emyoulation
Copy link

@DavidMStraub
Copy link
Member

Thanks! Makes perfect sense to me. Tagging @cdhorn as well who implemented reports.py.

@cdhorn
Copy link
Collaborator

cdhorn commented Feb 5, 2023

Yes, thanks and agree it makes sense. While I added the validation code there to try to sanity check things I clearly did not make time to test every option for every report.

Can you create a PR for David to merge?

@jeffasuk
Copy link
Contributor Author

jeffasuk commented Feb 6, 2023

I'm afraid I'm a GitHub newbie, so I'll need some help. I've cloned the repo, created a (local) branch, made the change in that branch, and committed it. I now have to push it.
push -n https://github.com/gramps-project/gramps-webapi.git
gets me a 403 response (permission denied). I guess I just don't have write permission on the repo. Or do I have to do something else? (e.g. make a remote branch?)

@DavidMStraub
Copy link
Member

Hi, assuming your clone is origin and your new branch is called fix_report, try

git push -u origin fix_report

That should do the trick and Github should automatically ask you if you want to open a PR.

@jeffasuk
Copy link
Contributor Author

jeffasuk commented Feb 6, 2023

$ git push -u origin handle_colon_in_report_option
Username for 'https://github.com': jeffasuk
Password for 'https://jeffasuk@github.com':
remote: Permission to gramps-project/gramps-webapi.git denied to jeffasuk.
fatal: unable to access 'https://github.com/gramps-project/gramps-webapi.git/': The requested URL returned error: 403

Although the prompt is "Password", I'm using a "personal access token (classic)".
A push to one of my own repos worked OK, so it does look like a user-level permission problem (or maybe "classic" token not accepted in your repo?)

@DavidMStraub
Copy link
Member

Sorry, I missed that you wrote "I've cloned the repo" rather than "I've forked the repo". You can't push directly to this repo, you have to click the "fork" button first and then clone your fork rather than the original repo.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/configuring-a-remote-repository-for-a-fork

@jeffasuk
Copy link
Contributor Author

jeffasuk commented Feb 6, 2023

Success! Thanks. PR #335
It didn't ask me about creating a PR, but it did give me a URL for doing that, which was good enough.

@DavidMStraub
Copy link
Member

Thanks for the contribution! Fixed by #335.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants