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

Add feature "[data-export] Delete button" #174

Merged
merged 2 commits into from
Oct 31, 2023
Merged

Add feature "[data-export] Delete button" #174

merged 2 commits into from
Oct 31, 2023

Conversation

ogomezba
Copy link
Contributor

@ogomezba ogomezba commented Oct 8, 2023

Hi! I've created this PR to implement the feature described in #134.

Test evidence after opening the test-framework page:
Screenshot 2023-10-08 a las 22 27 08

Thank you!

@tprouvot
Copy link
Owner

Thank you very much @ogomezba !
This is great job 👍

Could you use the "linkTarget" variable used in props to define if the link must open in a new tab or the current one ?
The goal is to let users decide it based on a custom setting

image

@tprouvot
Copy link
Owner

Optional parts which can be done after (and if you want to) :

  • add tests in data-export-test and data-import-test to cover this new functionality
  • delete only filtered records from data export page (WIP here)

- Tests for the "Delete records" functionality
- Now using the flag to determine if open the data-import deletion
in a new window
- Including compatibility to only delete filtered records
@ogomezba
Copy link
Contributor Author

Thank you @tprouvot for the great suggestions! I was not sure about what was the best way to include the changes in the other branch for the filtered records to make them work with my changes. I've included what I needed directly in my changes. Should I have merged the branch into mine instead?

Also, if you have any suggestions for the tests, please feel free to comment.

@tprouvot
Copy link
Owner

Hi @ogomezba
Thanks for the updates and sorry for the delay ⌛ !
I'll add those improvements in the CHANGES.md which is used to describe the release updates.

FYI when doing the tests on this feature, I created a new one pretty much linked to it, which goal is to .... undelete records 😄

Good job on this PR 👏 happy to welcome you to the team !

@tprouvot tprouvot merged commit 4d870db into tprouvot:releaseCandidate Oct 31, 2023
@ogomezba
Copy link
Contributor Author

@tprouvot Thank you for the effort that you are putting and the feedback! Really glad to contribute to such awesome tool that I use on a daily basis.

I think I will have some time this week to work on the undelete feature. Thank you for letting me know!

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