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 #363: Added fix for template soft deletes #372

Conversation

CarlosOVillanueva
Copy link

Signed-off-by: cvillanueva@equinix.com cvillanueva@equinix.com

Description

This fix removes the unique attribute from the name column inside of the template table. This attribute was prevent new templates from being added - if they shared the same name. The requirement we wish to fulfill is:

  1. When a user deletes a template, they should be able to create a new template with the same name.

The problem is that we do not hard delete records. Instead, we add a datestamp inside of the deleted_at column and then filter any records where deleted_at is not null (getTemplate).

We still need to be able to prevent users from creating new records with the same name, if the records are still active. To meet this requirement, I added the getTemplate() function to the createTemplate() function and passed the filter of name. If a record is returned, then return an error that the record exists; otherwise, proceed with creating the record.

Why is this needed

#363

Fixes: #363

How Has This Been Tested?

  • Ran the updated sql query inside of postgres. Validated that the uniqueness field was removed
  • Ran db test (db unit tests are part of a different PR), and validated the requirements

How are existing users impacted? What migration steps/scripts do we need?

  • Users must run db migration

Checklist:

I have:

  • updated the documentation and/or roadmap (if required)
  • added unit or e2e tests
  • provided instructions on how to upgrade

@CarlosOVillanueva CarlosOVillanueva changed the title added fix for template soft deletes FIX #363: Added fix for template soft deletes Dec 3, 2020
@codecov
Copy link

codecov bot commented Dec 3, 2020

Codecov Report

Merging #372 (b9b1849) into master (86057d3) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #372   +/-   ##
=======================================
  Coverage   24.74%   24.74%           
=======================================
  Files          14       14           
  Lines        1277     1277           
=======================================
  Hits          316      316           
  Misses        940      940           
  Partials       21       21           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 86057d3...b9b1849. Read the comment docs.

Signed-off-by: cvillanueva@equinix.com <cvillanueva@equinix.com>
@parauliya
Copy link
Contributor

Hi @CarlosOVillanueva ,
This fix is already covered in PR #371 which is under review.

@gianarb
Copy link
Contributor

gianarb commented Dec 4, 2020

Thanks @CarlosOVillanueva but as @parauliya pointed out he was working at the same bug and his PR goes a bit further ahead. Sorry for the waste of time, my feedback is to check the issue at first because in there you can quickly spot if somebody is working at it already

@gianarb gianarb closed this Dec 4, 2020
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.

Creating a template sharing the same name as a deleted template causes an error
3 participants