Skip to content

Add update_member and get_members_xlsx methods - #197

Open
alistair0adams wants to merge 25 commits into
Olen:mainfrom
alistair0adams:update-member
Open

alistair0adams wants to merge 25 commits into
Olen:mainfrom
alistair0adams:update-member

Conversation

@alistair0adams

Copy link
Copy Markdown

Added two methods

  1. update_member() which will POST updated member data back to Spond
  2. get_members_xlsx() which will download the Excel sheet that can be manually downloaded from the Spond website. This can easily then be converted into a Dataframe.

@elliot-100

elliot-100 commented Mar 26, 2025 •

Copy link
Copy Markdown
Collaborator

Thanks for this @alistair0adams .

Are you OK to add the missing return annotation? Should be JSONDict.

@alistair0adams alistair0adams left a comment •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JSONDict annotation added

@elliot-100

Copy link
Copy Markdown
Collaborator

Sorry @alistair0adams have been on other projects. Will try to test in next 1-2 weeks.

@elliot-100 elliot-100 self-assigned this Apr 18, 2025
@elliot-100
elliot-100 self-requested a review April 18, 2025 11:29
@Olen

Olen commented May 14, 2026

Copy link
Copy Markdown
Owner

Thanks @alistair0adams — preparing a 1.2.1 release sweep and would love to bring these methods in. Before merging, this PR needs a quick refresh:

  1. Rebase against main — the branch is currently flagged as conflicting after recent merges.
  2. Docstring fixups:
    • update_member: "Updated memberi" typo (extra i).
    • update_member: references get_personb() — should be get_person().
  3. Add a test for at least one of the two methods. The existing tests/test_spond.py::TestExportMethod::test_get_export is a close shape for get_members_xlsx and easy to adapt; update_member can follow test_change_response's mocking pattern.

If you're short on time and want to land just the cleanup + rebase, I'm happy to take #197 over by pushing to your branch (or open a follow-up PR that picks your commit). Let me know what works best.

@Olen Olen mentioned this pull request May 14, 2026
2 of 3 tasks
@alistair0adams

Copy link
Copy Markdown
Author

Sorry for the non resposonse earlier. I think we're all suffering working on other things. This is a heads up that I have done the rebase, written two tests, fixed the typos, but have a few test to do before saying it's ready to have another go at at PR. Please be patient, lots of things here that I had to learn that are not part of my daily life.

@alistair0adams

Copy link
Copy Markdown
Author

Looks like this is ready to go. It seems to have passed all the test so hoping I've got everything right.

@Olen
Olen requested a balanced review from Copilot October 9, 2026 20:17

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The payments API is outside the stated scope, lacks tests, has an incorrect return contract, and the changed Python files fail repository formatting conventions.

4 open findings
What changed in this PR

Adds member update/export APIs, plus an additional received-payments API.

Changes:

  • Adds member updating and XLSX export methods with tests.
  • Adds received-payment retrieval and caching.
  • Documents APIs and provides usage examples.
File Description
spond/​spond.py Implements the three API methods.
tests/​test_spond.py Tests member updating and XLSX export.
README.md Documents the new methods.
examples/​update_member.py Demonstrates updating member fields.
examples/​payments.py Demonstrates exporting payments.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread examples/update_member.py Outdated
Comment thread spond/spond.py Outdated
Comment thread README.md Outdated
Comment thread examples/update_member.py Outdated
@elliot-100

Copy link
Copy Markdown
Collaborator

@alistair0adams not sure if you are familiar with ruff?
You should be able to resolve most of the issues reported with:

poetry run ruff format
poetry run ruff check --fix

(We should document this in contribution guidelines).

Have to say, adding new (payments) features to an in-progress PR isn't a good idea. Can I suggest, to get this done, just remove them to a future PR?

@alistair0adams

alistair0adams commented Oct 12, 2026 •

Copy link
Copy Markdown
Author

Thanks @elliot-100. I'd used ruff when it complained earlier but not with the format option. I've taken out the payments feature. It looks clean to me but as I've found out looks can be deceiving.

@Olen

Olen commented Oct 12, 2026

Copy link
Copy Markdown
Owner

Your effort is highly appreciated.

alistair0adams and others added 3 commits October 13, 2026 12:33
Fix typo in update_member() documentation.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

This branch has not been deployed

No deployments
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

Successfully merging this pull request may close these issues.

4 participants