When two users attempt to add items to the catalog, both entry forms are assigned the same catalog number, and when they submit, whoever submits second gets an error.
This doesn't happen with node ids, since it's an auto-increment field.

I'm not too familiar with auto-increment, but would changing station_catalog.number to auto-increment solve this problem?

CommentFileSizeAuthor
#11 station_593180.patch3.26 KBtim.plunkett
#9 station_593180.patch3.26 KBtim.plunkett

Comments

drewish’s picture

drewish commented

humm... that's definitely a problem. i didn't use an serial integer field because i wanted people to be able to assign arbitrary numbers. we had a huge library that was already numbered.

drewish’s picture

drewish commented
Title: Auto-Increment of Station Catalog Number » Easy to get duplicate Station Catalog Numbers
Category: feature » bug
Priority: Normal » Critical
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented

This is becoming more and more of an issue. I can't figure out exactly how to remove all the checks to station_catalog.number, because I'm not sure which ones are contrived and which will break all the code.
Any help on this?

drewish’s picture

drewish commented

humm... i'm not sure what the best way to approach this would be... maybe allow the value to be blank and if so a TABLE LOCK; SELECT MAX(); INSERT; UNLOCK?

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented

I haven't had a chance to deal with this, but it's increasingly becoming a huge issue. Any ideas?

drewish’s picture

drewish commented

maybe the best thing to do is to not add the new id when building the form and if one isn't entered we generate it on submit?

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented

That sounds like a great way to do it. Do you have time to work that out, or should I take a crack at it?

drewish’s picture

drewish commented

if you have a chance i'd appreciate it. i'm pretty tied up.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Active » Needs review
StatusFileSize
new station_593180.patch3.26 KB

I think I did what you suggested in #6.

Made station_album.number not required, default=''.
Had to alter station_catalog_album_validate() to not set the error for '', not sure if that's done the best way.

Then added station_catalog_album_next_number() to station_catalog_album_node_form_submit().

drewish’s picture

drewish commented

Not quite sure why/what's changing in the validation code. Also there's some coding standards issues. You're missing some white space, take a look at http://drupal.org/coding-standards . Probably also want to add some isset() tests—I might have overlooked some.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
StatusFileSize
new station_593180.patch3.26 KB

Within the validation code:
The addition of if ($node->album['number'] != '') checks to see if the number field was left blank.
While empty() would catch empty strings, it would also catch "0", which should be a valid entry.
The rest of the lines changed are just indents because of the outer if statement.

I don't know where to add any isset() tests, since all 3 cases (empty string, numeric, non-numeric) are covered.

I also fixed the whitespace errors, and ran it through coder.

drewish’s picture

drewish commented

I'm okay with calling 0 an invalid id.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented

Are you saying the logic should be revised to disallow 0? Because right now it's considered valid.

drewish’s picture

drewish commented

i was saying that i don't see a need for 0 as a valid number and if it simplifies the logic all the better.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs review » Reviewed & tested by the community

I would say that after further thought, the current logic makes sense and is the simplest solution, so I'd say RTBC.

drewish’s picture

drewish commented
Status: Reviewed & tested by the community » Fixed

Thanks, committed to HEAD.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.