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.

Sign up for GitHub

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

Jump to bottom

[DQT] add delete feature #8078

Merged
merged 23 commits into from
Mar 17, 2023
Merged

[DQT] add delete feature #8078

merged 23 commits into from
Mar 17, 2023

Conversation

kongtiaowang
Copy link
Contributor

@kongtiaowang kongtiaowang commented Apr 4, 2022 •
edited
Loading

Brief summary of changes

Add delete feature for DQT
Add tools/dqt_delete_saved_query.php

Testing instructions (if applicable)

  1. Save a private or global query.
  2. click delete button to delete or using dqt_delete_saved_query.php to delete the new saved query.

@kongtiaowang kongtiaowang added the Category: Feature PR or issue that aims to introduce a new feature label Apr 7, 2022
maltheism
maltheism requested changes Oct 13, 2022
View reviewed changes
Copy link
Member

@maltheism maltheism left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @kongtiaowang the feature works great!

modules/dqt/ajax/DeleteDoc.php Show resolved Hide resolved
modules/dqt/ajax/saveQuery.php Show resolved Hide resolved
modules/dqt/jsx/react.savedqueries.js Outdated Show resolved Hide resolved
@maltheism maltheism added the State: Needs work PR awaiting additional work by the author to proceed label Oct 13, 2022
@kongtiaowang kongtiaowang reopened this Oct 22, 2022
@kongtiaowang kongtiaowang removed the State: Needs work PR awaiting additional work by the author to proceed label Oct 22, 2022
maltheism
maltheism requested changes Oct 28, 2022
View reviewed changes
Copy link
Member

@maltheism maltheism left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @kongtiaowang I reviewed again and the only thing I see left is the eslint-disable-line(s) that should probably be removed? Also this new delete feature should probably be added to the changelog file of Loris.

modules/dqt/jsx/react.tabs.js Outdated Show resolved Hide resolved
@maltheism maltheism added the Passed manual tests PR has been successfully tested by at least one peer label Oct 31, 2022
@kongtiaowang
Copy link
Contributor Author

@maltheism Thanks

xlecours
xlecours requested changes Oct 31, 2022
View reviewed changes
modules/dqt/ajax/DeleteDoc.php Outdated Show resolved Hide resolved
modules/dqt/jsx/react.savedqueries.js Outdated Show resolved Hide resolved
modules/dqt/ajax/DeleteDoc.php Outdated Show resolved Hide resolved
modules/dqt/jsx/react.tabs.js Outdated Show resolved Hide resolved
tools/dqt_delete_saved_query.php Show resolved Hide resolved
xlecours
xlecours requested changes Oct 31, 2022
View reviewed changes
modules/dqt/ajax/DeleteDoc.php Outdated Show resolved Hide resolved
modules/dqt/jsx/react.savedqueries.js Outdated Show resolved Hide resolved
@kongtiaowang kongtiaowang removed the Passed manual tests PR has been successfully tested by at least one peer label Oct 31, 2022
@kongtiaowang
Copy link
Contributor Author

Hi @maltheism Could you test it again? Thanks

@maltheism
Copy link
Member

maltheism commented Oct 31, 2022 •
edited
Loading

HI @kongtiaowang I can still delete successfully. Passed manual tests!

@kongtiaowang
Copy link
Contributor Author

Thanks @xlecours @maltheism

@kongtiaowang kongtiaowang added the Passed manual tests PR has been successfully tested by at least one peer label Oct 31, 2022
@laemtl laemtl added this to the 25.0.0 milestone Feb 28, 2023
@kongtiaowang kongtiaowang reopened this Feb 28, 2023
@laemtl laemtl assigned racostas and unassigned xlecours Mar 14, 2023
@racostas racostas self-requested a review March 16, 2023 19:45
racostas
racostas approved these changes Mar 16, 2023
View reviewed changes
Copy link
Contributor

@racostas racostas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@driusan driusan merged commit ae89857 into aces:main Mar 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@racostas racostas racostas approved these changes

@xlecours xlecours Awaiting requested review from xlecours

@driusan driusan Awaiting requested review from driusan

@maltheism maltheism Awaiting requested review from maltheism

Assignees

@racostas racostas

Labels
Category: Feature PR or issue that aims to introduce a new feature Passed manual tests PR has been successfully tested by at least one peer
Projects
None yet
Milestone
25.0.0
Development

Successfully merging this pull request may close these issues.
6 participants
@kongtiaowang @maltheism @driusan @xlecours @racostas @laemtl