It’s really confusing when you have two canned replies with the same title. Can anyone prevent this from happening?
I agree, that could be confusing. Working on a PR for this now
Is it ok if the plugin returns an error message like: Error: Duplicate title
?
My intent is for the API to return an error upon submitting the new reply and for the UI Components to parse the error and return a pretty error message.
Currently I was thinking of: “Um oh! This title has already been used for another canned reply on your account. Please choose a different title.”
Just an update on my progress:
- It’s my understanding that I need to fetch the list of shared canned replies, loop over every reply, and throw an error to the user if the title matches any of the replies. This seems inefficient in my mind, but computers are very fast, and it’s unlikely a forum will have 1000s of canned replies so at worst this is an extra database query on add reply and edit reply.
- I need to modify the UI to detect the API rejecting to insert the reply and to display my error in a pretty way.
- Still allow adding normal canned replies.
That’s one way you could do it. But if you make a field UNIQUE any attempt to INSERT a duplicate value would cause a fail.
That’s true but what about customers who already have two entries with duplicate titles. The migration for them would fail.
An approach may be to use a Sidekiq job to do the checking and make any duplicates unique and send a PM notifying the site admin of the changes. Though I should note I haven’t done this when changing a field to unique, so I can’t say how well it’ll mesh.
We still have to address any sites who have two canned replies with the same title before setting a unique key.
I think that’s what @justin was trying to suggest a fix for. On update, prior to making the field unique,
- find non-unique titles.
- Add a numeral or something
- PM the admin the list of canned replies that had their titles modified.
- Make the field unique.
An update on my progress:
The API now only adds the reply if the title isn’t already in use.
https://github.com/nsuchy/discourse-canned-replies/commit/6fe13c5b868e94d273c3abda1d6b3d006c0a7efd
I’m not exactly sure where to start with the UI and displaying an error. Currently (this can be easily changed), the API returns {error: 'Title already in use!'}
but the UI doesn’t react to it and the status code is still 200. While debugging a few times I got status code 500s and the UI did react to that as expected how can I go about sending a specific status code then changing the error message? Any guidance on that would be appreciated.
@jtbayly I’ll look into the actual migration steps and implementation soon
@sam I had an idea, how do you feel about the new reply model checking if the title is taken already and disabling the button if it is? Similar to what Discourse registration does.
Another progress update
The controller returns a proper error message now. (https://github.com/nsuchy/discourse-canned-replies/commit/3da1023aa7a9577b6ec6dd9b2b4e4e8b0b6154be)
Can someone review my pull request at https://github.com/discourse/discourse-canned-replies/pull/45? Thanks
On setting unique keys
There was some discussion on setting a unique key. Everything is currently stored in a single row in the database which is updated. Since most forums will only use a handful of canned replies this does not introduce a huge performance penalty yet. However this does prevent us from setting a unique key in the database. Are plugins “allowed” to create their own tables in the database? This would require a major change to the plugin if we want to utilize unique keys which are enforced by the database server and would be another discussion on if/when/how that should be done.
Screenshot of query: