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

fix(request-termination) allow null value for body or message #3784

Closed
wants to merge 1 commit into from

Conversation

shashiranjan84
Copy link
Contributor

Summary

Allow user to unset either message or body.

Full changelog

  • update schema self_check to support null value for message or body
  • add relevant spec

Allow user to unset either message or body.

Changes:
- schema self_check update to support null value
- add relevant spec
@thibaultcha
Copy link
Member

In the next branch, we already ported plugins to the new DAO, and we currently are in the process of convert plugins schemas to the new schema library (#3778). The new schema library/Admin API endpoints should support JSON null un-setting out of the box. Still, it is nice to have tests for this.

  1. We should probably wait until Feat/plugins schemas to new db #3778 is done, before reworking this PR to reflect the desired changes, and introduce those new tests
  2. There should be a test that makes two subsequent requests, the first one un-setting message, and the second one un-setting body). That should not be possible (as per the "either one of message or body should be set" error).

@thibaultcha thibaultcha added pr/wip A work in progress PR opened to receive feedback and removed pr/please review labels Sep 21, 2018
@bungle bungle changed the title fix(request-termination) allow null vlaue for body or message fix(request-termination) allow null value for body or message Sep 21, 2018
@thibaultcha
Copy link
Member

"WIP" label applied to reflect the need for #3778 to be merged first, before porting those changes to the new schema. Keeping the PR open in the meantime!

@shashiranjan84
Copy link
Contributor Author

There should be a test that makes two subsequent requests, the first one un-setting message, and the second one un-setting body). That should not be possible (as per the "either one of message or body should be set" error).

good catch, wondering if it possible to figure out what plugin state saved in DB from self check?

@CLAassistant
Copy link

CLAassistant commented Apr 9, 2020

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Shashi Ranjan seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@hishamhm hishamhm closed this Sep 18, 2020
@hishamhm hishamhm deleted the fix/rt-schema branch September 18, 2020 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/wip A work in progress PR opened to receive feedback
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants