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

Support changing estimate state #289

Merged
merged 4 commits into from
Nov 2, 2023
Merged

Support changing estimate state #289

merged 4 commits into from
Nov 2, 2023

Conversation

MultiSuperFreek
Copy link
Contributor

When updating an estimate, changing it's state is not allowed and silently ignored. Changing the state of an estimate requires a separate API call. The method added exposes this endpoint through the PHP Client.

When updating an estimate, changing it's state is not allowed and silently ignored. Changing the state of an estimate requires a separate API call. The method added exposes this endpoint through the PHP Client.
@MultiSuperFreek
Copy link
Contributor Author

MultiSuperFreek commented Oct 30, 2023

Sorry about the formatting issues, I have fixed them.

@MultiSuperFreek
Copy link
Contributor Author

This would fix #250

@robindirksen1
Copy link
Contributor

@MultiSuperFreek out of curiosity: what if Moneybird adjusts the validation? So then the package has to be updated again first to match this server-side validation?

@MultiSuperFreek
Copy link
Contributor Author

@robindirksen1 After checking with my colleague we've confirmed that Moneybird already returns a 400 Bad Request when an invalid state is given (I was under the impression this was not the case, and the API returned an new Estimate with the given state ignored), so this validation does little more then increasing maintenance burden. I'll remove the client side check!

@robindirksen1
Copy link
Contributor

LGTM ✅

@stephangroen or @sanderlissenburg, can you take a look at this PR?

@stephangroen stephangroen merged commit b2ffa6c into picqer:main Nov 2, 2023
@MultiSuperFreek MultiSuperFreek deleted the patch-1 branch November 2, 2023 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants