Code review comment for lp:~mhaulo/mixxx/allow-playlist-and-crate-renaming

Revision history for this message
Mika Haulo (mhaulo) wrote :

>
> thank you very much for your contribution. I have reviewed your branch and
> found a bug: Assume you have two playlists "A" and "B". Renaming "B" to "A"
> will not fail and results in two playlists "A" and "A". Therefore, you must
> check if the renamed playlist name exists or not.

Ah, how on earth did I forget that... anyway, a fix for this should be available in my branch now.

I also changed the behaviour a little bit. First of all, the user input is trimmed to eliminate extra whitespace. This effectively prevents whitespace-only names. If the name didn't change from the old name, the function simply returns doing nothing more. And the whole thing is in a loop that keeps popping up an input dialog until the user either gives a valid name or cancels the action.

I'd suggest this kind of behaviour also to be added to the new crate/playlist feature. What do you think?

> Furthermore, I would like
> see an UNIQUE constraint on attribute <name> in the <playlist table> (if other
> developers agree).

I agree with this.

« Back to merge proposal