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

Implement option to copy a tag #924

Merged
merged 21 commits into from
Nov 25, 2024
Merged

Implement option to copy a tag #924

merged 21 commits into from
Nov 25, 2024

Conversation

snake14
Copy link
Contributor

@snake14 snake14 commented Nov 14, 2024

Description:

This PR implements the ability to copy tags. It tries to reuse existing triggers and variables when copying references from the tag. If no matches exist, any dependencies will be copied as well.

Fixes: #615

Review

@snake14 snake14 marked this pull request as ready for review November 19, 2024 03:46
Copy link
Contributor

@AltamashShaikh AltamashShaikh left a comment

Choose a reason for hiding this comment

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

@snake14

  1. After copying a tag, there is no notification that its a success or failure.
  2. For tags when we click copy the default container should be the current container

I also faced 1 issue of copy tag throwing and error "This name is already in use" when copying a same tag twice in an another container

Copy link
Contributor

@AltamashShaikh AltamashShaikh left a comment

Choose a reason for hiding this comment

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

@snake14 The copy notification is getting hidden, if preview mode is enabled

Without preview mode

image

With preview mode, no notification message is displayed

image

@snake14
Copy link
Contributor Author

snake14 commented Nov 19, 2024

Thank you for the good review @AltamashShaikh .

It looks like I'm going to have to refactor things so that the notifications are handled via Vue instead of PHP, because the PHP notification just doesn't appear to be reliable enough.

I thought that I had it selecting the current container by default, but I'll make sure.

I have test cases around copying to the same location twice, but I guess I only did that for triggers and variables. I'll add one for tags too and fix the name issue. 👍

Copy link
Contributor

@AltamashShaikh AltamashShaikh left a comment

Choose a reason for hiding this comment

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

Looks good now 👍

@snake14 snake14 merged commit 6accb07 into 5.x-dev Nov 25, 2024
5 checks passed
@snake14 snake14 deleted the PG-2738-copy-tag-option branch November 25, 2024 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Enhancement] Add "Copy tag" option in Matomo Tag Manager
2 participants