Skip to content

Check name duplicate#318

Merged
birm merged 24 commits intocamicroscope:developfrom
amritansh22:develop
Apr 5, 2020
Merged

Check name duplicate#318
birm merged 24 commits intocamicroscope:developfrom
amritansh22:develop

Conversation

@amritansh22
Copy link
Copy Markdown
Contributor

@amritansh22 amritansh22 commented Apr 4, 2020

In the model app, when the name of the uploaded models are same, it creates confusion later while choosing the model and error while removing the model. This PR fixes these issues.

So, now when we enter repeated name, we see an error as:


image

@amritansh22 amritansh22 mentioned this pull request Apr 4, 2020
@amritansh22 amritansh22 marked this pull request as ready for review April 4, 2020 19:27
@mgautam98
Copy link
Copy Markdown
Contributor

@amritansh22 it is not working if I refresh the page remember that the models are persistent in the memory therefore, this feat. also needs to be persistent.

Also, it is not working if I don't refresh, it still uploads the model becaue you need to return after catching the error. Here, it displays it and then continues the execution.

@amritansh22
Copy link
Copy Markdown
Contributor Author

@mgautam98 Thanks a lot for pointing out the mistakes. I have fixed them.

@birm birm self-requested a review April 5, 2020 18:02
Copy link
Copy Markdown
Member

@birm birm left a comment

Choose a reason for hiding this comment

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

Thanks for getting this together! I'll merge it before we make another conflict ;)

@birm birm merged commit 48b3f98 into camicroscope:develop Apr 5, 2020
@mgautam98
Copy link
Copy Markdown
Contributor

@birm are you sure it's working. The namearr is not being populated with all the model names. Though, model name is saved as soon as you refresh or go to other pages the namearr is again initialised to empty array.

@birm
Copy link
Copy Markdown
Member

birm commented Apr 5, 2020

Hm, I may have missed something when testing... If there's a a fix to this fix, we'll certainly welcome a PR.

@leoarc leoarc mentioned this pull request Apr 13, 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.

3 participants