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

Adding duplicate feature should throw message and revert addition in TestScriptor page #87

Closed
lvanvugt opened this issue Jan 30, 2021 · 7 comments
Assignees
Labels
bug Something isn't working needs test defined needs testing Ready for testing question Further information is requested version A To be picked up right away
Milestone

Comments

@lvanvugt
Copy link
Contributor

lvanvugt commented Jan 30, 2021

When adding a duplicate feature a message should be thrown and the added feature reverted in TestScriptor page.

To Reproduce
Steps to reproduce the behavior execute the following test scenario

No. Feature Scenario
0095 Adding Feature Adding duplicate Feature

in atdd-test-scenarios-and-results.xlsx

Result
The Feature control on the ATDD.TestScriptor page remains while (rightly) no test codeunit has been created (as it already existed).

Expected result
The Feature control on the ATDD.TestScriptor page should be reverted and a message should be shown, telling "Feature already exists. Please update your feature definition so it is unique"

Belonging to #61.

@lvanvugt lvanvugt added the bug Something isn't working label Jan 30, 2021
@lvanvugt lvanvugt added the version A To be picked up right away label Jan 30, 2021
@lvanvugt lvanvugt added this to the beta release milestone Jan 31, 2021
@DavidFeldhoff
Copy link
Collaborator

@martonsagi : With 784ed01 I return now that a feature already exists.
That works, so that the feature is not inserted. But let's say I create a second time the feature "First test object". Then I return "That feature already exists.".
So I want to add "First test object 2" instead of "First test object". But then I'm triggered as if it would be a Renaming of the feature which is not true as it wasn't inserted in the first place. Could you have a look at it?

@martonsagi
Copy link
Collaborator

@DavidFeldhoff What is in the message object that you send back to the UI in this case?

@DavidFeldhoff
Copy link
Collaborator

I made a change here, so that I now return false in the procedure correctly and the "userCancelledSaving" variable is true. But the issue of my previous comment remains.
b3d67b3#diff-0fc540fe39809847cade5bbff4841e10f7963991d25714e61c7cb5d458736f31R220
https://github.com/fluxxus-nl/ATDD.TestScriptor.VSCodeExtension/blob/master/web-ui/src/app.ts#L45

@lvanvugt
Copy link
Contributor Author

This is clearly no finished

@DavidFeldhoff
Copy link
Collaborator

@martonsagi I'll put it on your name only as I return the correct state (in my eyes). If I'm missing something or if I can help you, let's have a chat.

@martonsagi
Copy link
Collaborator

#125 has been implemented by clearing the value after failure.
The renaming popup showed earlier becuase the list values are simple text values, there is no state property handling from where we would know that a certain value was invalidated previously. That would need a redesign of the feature pane.

@martonsagi martonsagi added the question Further information is requested label Apr 15, 2023
@lvanvugt lvanvugt assigned lvanvugt and unassigned martonsagi Apr 16, 2023
@lvanvugt
Copy link
Contributor Author

Agree with closing this issue because of fix for #125.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs test defined needs testing Ready for testing question Further information is requested version A To be picked up right away
Projects
None yet
Development

No branches or pull requests

3 participants