Newsletter categories weight is not saved [#1930176]

The newsletter categories administration page allows ordering categories, but changes are not saved when submitting the form. The available weights (even though they are ignored) are also out of range compared to the taxonomy term edition form (which also allows ordering the categories).

Here's a patch that fixes both problems.

Comment	File	Size	Author
#1	0001-1930176-Newsletter-categories-weight-is-not-saved.patch	2.04 KB	SeeSchloss

Comments

Comment #1

SeeSchloss commented 28 February 2013 at 14:25

Status	File	Size
new	0001-1930176-Newsletter-categories-weight-is-not-saved.patch	2.04 KB

Here's the patch

Comment #2

28 February 2013 at 14:30

Status:

Needs review

» Needs work

The last submitted patch, 0001-1930176-Newsletter-categories-weight-is-not-saved.patch, failed testing.

Comment #3

berdir

German

Switzerland

commented 1 March 2013 at 13:19

According to the tests, the weight isn't always available.

Also, http://api.worldempire.ch/api/simplenews/includes%21simplenews.admin.inc... already seems to take care of the weight when saving a single category it seems, it's a bit unfortunate that this change would mean that we always load and save the term twice.

Maybe we should instead change http://api.worldempire.ch/api/simplenews/includes%21simplenews.admin.inc... to directly load, update and save the term instead?

Comment #4

SeeSchloss commented 1 March 2013 at 13:40

Actually, there's another problem the simplenews_admin_category_form - submitting this form just always sets the weight to 0, through a hidden value, see http://api.worldempire.ch/api/simplenews/includes!simplenews.admin.inc/f...
This should be fixed too, but I had missed it at first.

Rather than adding term saving to the simplenews_admin_categories_submit, I think it would make more sense to remove it from simplenews_admin_category_form_submit and move all of it to simplenews_category_save.

I imagine the test failures are mostly due to tests forgetting to set a weight value where it would be set in normal usage (since simplenews_category_save, the only method I really changed in a significative way, is only ever called from the two form submits we are talking about here), but I don't have a machine where I can run tests right now to check that.

Just to be clear, I also don't think it's a good idea to just set a default weight when saving the term to make the tests pass, it wouldn't be fixing the real problems.