Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#1763 closed bug (fixed)

Preferences dance

Reported by: planetcruiser Owned by: shevek
Priority: major Milestone: 1.4
Component: BW Profile Keywords:
Cc: mahouni, jsfan, planetcruiser

Description

Issue:

Reproduce:

  1. Set "I want to receive periodic news about BeWelcome by mail" to yes
  2. It will appear near the bottom of the page
  3. Set it to "no"
  4. It will appear near the top of the page
  5. The same thing happens with the new "Show profile visits" preference

Expected behaviour:

  • All items stay at their position, regardless their value/setting

Solution:

  • Investigate how items are sorted
  • Fix sorting to be consistent
  • Consider adding a position column in the database, so the order can be controlled

Related tickets:

Attachments (3)

preferences.sql (14.9 KB) - added by planetcruiser 6 years ago.
dump from live db
preferences_update.sql (1.1 KB) - added by shevek 6 years ago.
patch1763.diff (466 bytes) - added by shevek 6 years ago.

Download all attachments as: .zip

Change History (30)

comment:1 Changed 6 years ago by shevek

Ordering happens by value. Instead it should be sorted by preferences.id. Adding easy and 1.4 as keywords.

comment:2 Changed 6 years ago by shevek

  • Keywords easy 1 4 added

comment:3 Changed 6 years ago by shevek

  • Keywords 1.4 added; 1 4 removed

comment:4 Changed 6 years ago by shevek

  • Keywords easy 1.4 removed
  • Milestone changed from unassigned to 1.4
  • Owner set to shevek
  • Status changed from new to assigned

Order by preferences.id. Should make life of support easier as well.

Commit: https://gitorious.org/bewelcome/rox/commit/85758a408acafcfcb6d683464cc1af1b55050cc5

comment:5 Changed 6 years ago by mahouni

  • Cc mahouni added

comment:6 Changed 6 years ago by mahouni

tested locally: works for me. code reviewed.

comment:7 Changed 6 years ago by crumbking

tested lcoally. works for me, too.

comment:8 Changed 6 years ago by shevek

As preferences now stay put we should think about the sequence and if we can set one.

It would be nice to have the daylight saving setting and the time zone setting next to each other as an example.

@jsfan and planetcruiser: The test db contains a different preferences table than the production system could you please attach a dump of the production system to this ticket?

comment:9 Changed 6 years ago by shevek

  • Cc jsfan planetcruiser added

Changed 6 years ago by planetcruiser

dump from live db

comment:10 Changed 6 years ago by planetcruiser

dump added, the dev dump should be updated with this

Changed 6 years ago by shevek

Changed 6 years ago by shevek

comment:11 Changed 6 years ago by shevek

Thanks.

Timezone and daylight setting obviously were added at the same time. So they are next to each other.

I'd like to group 'Display of forum first page' and 'Groups only' and 'Notifications about local events' and 'I want to receive periodic news..'.

'Show profile visits' and 'Public profiles' might be good together as well.

So a position column would be better than the current simple sort on Id.

The attached SQL would take care of this. Language and public profile are handled independently in the template so won't be affected by the changes.

The diff would sort accordingly.

Could someone please test this?

comment:12 Changed 6 years ago by crumbking

Why we are working on it. Could we get the HTML out of the DB? We could add a simple build/members/preferences.page.php and move all the stuff there. Why do we work all the time in the database?

comment:13 Changed 6 years ago by shevek

The column EvalString? isn't used in Rox at all. Only referenced on some very old bw stuff.

comment:14 Changed 6 years ago by jsfan

Deployed on alpha.

comment:15 Changed 6 years ago by shevek

Preferences stay put on alpha (but someone else has to confirm as I changed it). But I noticed that after saving the preferences the page is always redirected to the preferences in the preferred language. Rather odd. I raise a new ticket for that.

comment:16 Changed 6 years ago by shevek

  • Status changed from assigned to testing

comment:17 Changed 6 years ago by planetcruiser

  • Resolution set to fixed
  • Status changed from testing to closed

yep, looking good

comment:18 Changed 6 years ago by planetcruiser

oh, wait, are we introducing the 'position' column with this or not? if yes, this ticket needs to be reopened.

comment:19 Changed 6 years ago by shevek

  • Resolution fixed deleted
  • Status changed from closed to reopened

@planetcruiser: I'd like to add the position column. So if you could test the attached files would be cool.

comment:20 Changed 6 years ago by jsfan

  • Status changed from reopened to local_testing

comment:21 Changed 6 years ago by crumbking

What to test here? The preferences doesn't dance anymore. Something else? ;-)

comment:22 follow-up: Changed 6 years ago by crumbking

I executed the attached sql update dump. Seems to work.

What about tabs?

tab names: account / notifications /...?

Probably should go into another ticket.

comment:23 in reply to: ↑ 22 Changed 6 years ago by shevek

  • Status changed from local_testing to to_alpha

I executed the attached sql update dump. Seems to work. What about tabs?

tab names: account / notifications /...?

Probably should go into another ticket.

That change can wait for welen I'd say. We don't have that many preferences at the moment.

Commit: https://gitorious.org/bewelcome/rox/commit/b0b799bda730508ab6e47b7f39a8930dc451751c

Status change 'to_alpha'. Please make sure to update the DB before deploying this.

comment:24 Changed 6 years ago by shevek

  • Status changed from to_alpha to testing

Deployed to alpha. Please test.

comment:25 Changed 6 years ago by mahouni

tested on alpha. Works for me.

comment:26 Changed 6 years ago by shevek

  • Resolution set to fixed
  • Status changed from testing to closed

Just checked the grouping again. Looks good to me.

With mahouni's comment we can close this one.

comment:27 Changed 6 years ago by planetcruiser

tested ok. no more dance and grouping (time settings appear together)

Note: See TracTickets for help on using tickets.