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 deletion of configs #61

Merged
merged 2 commits into from
Apr 25, 2019

Conversation

vzDevelopment
Copy link
Contributor

Prior to this PR if you used the X button to delete a config for an app or function (as the interface intends) then it wouldn't make any changes. This is because the config data isn't included in the PUT request so the API doesn't think it's been changed. The correct way of deleting a config with the new V2 API is to set its value to an empty string. I have changed the code so it does this behind the scenes, whilst keeping the frontend appearance the same.

I have also separated the config logic into its own Vue component and shared it between apps and functions to remove the duplicated code.

To test:

  • Create an app with some config values
  • Use the delete button to delete some config values
  • Check using the UI or fn inspect that the config has been updated accordingly

I have also tested deleting a config and then adding it back prior to clicking submit and this works correctly.

This PR contributes to #52

With the new V2 API, to delete a config entry, you need to set the value
to an empty string. If you don't do this, the system doesn't realise you've
made any changes and therefore doesn't update the config. Update the
code to support this functionality.

Contributes to fnproject#52.
The Fn/App config was duplicated on both the App and Fn form. By moving
it to its own component it means we only have to maintain it in one
place. For example, I recently fixed the deletion of config lines.
Having one component to edit means I only need to apply the fixes to one
place.

As a consequence of this, it also means it fixes the deletion of config
points for Apps - prior to this change deleting the config row wouldn't
have any effect and you'd need to set the config value to an empty
string to delete it.
@carimura carimura merged commit ce639c9 into fnproject:master Apr 25, 2019
@carimura
Copy link
Member

sweet

@carimura
Copy link
Member

i moved too quickly. just tested and doesn't seem to be deleting config vars from UI...

@carimura
Copy link
Member

related: #63

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.

2 participants