Opened 6 years ago

Closed 6 years ago

#1849 closed bug (fixed)

Cannot edit post visibility when replying in the forum - but it works when editing

Reported by: pablobd Owned by: shevek
Priority: major Milestone: 1.4
Component: BW Forum Keywords: visibility 1.4 1.5
Cc:

Description

Here's the bug: If i hit reply in a given forum thread, for example http://www.bewelcome.org/forums/s318-BeLocal_5___The_role_of_Local_Volunteers_/reply I cannot edit forum visibility (i dont even see the field) BUT If I edit my post just after posting, for example: http://www.bewelcome.org/forums/edit/m21916 I get the option to set the visibility I don't know if this has been always this way, but looks like a big bug

Change History (37)

comment:1 Changed 6 years ago by beatnickgr

Ah! i was looking for this "visibility" button too.. I didnt know we have to edit the post to edit the visibility...

Last edited 6 years ago by beatnickgr (previous) (diff)

comment:2 Changed 6 years ago by shevek

Which is pretty annoying for all subscribers as they get informed that a post was edited.

comment:3 Changed 6 years ago by shevek

  • Keywords 1.4 1.5 added
  • Owner set to shevek
  • Status changed from new to assigned

I have a fix ready for this.

comment:4 Changed 6 years ago by mahouni

  • Milestone changed from unassigned to 1.4
  • Status changed from assigned to local_testing

comment:5 Changed 6 years ago by shevek

Just tested locally. Works fine for replies now.

Unfortunately the same isn't true for editing. Is there a way to find out if a forum post is a group post and in which group from the edit link?

comment:6 Changed 6 years ago by shevek

For some reason isset($this->_model->IdGroup?) didn't return true so the assignment of the group wasn't initialized correctly. After digging in the code I found that IdGroup? of the model is always set.

The following commit fixes the issue with the visibility settings for group post edits: https://gitorious.org/bewelcome/rox/commit/47ab17a9754cd96232bf51885284ed6cb796bb01

Please test locally.

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

I tested locally. Found a strange bug now. On one of your groups start a group only thread. Hit the reply button and the text of all "group only" posts below the reply form are not visible.

comment:8 Changed 6 years ago by crumbking

Okay if I understand correct if someone post group only all others (not part of the group) should not see the text but still the avatar, post date, etc.

Last edited 6 years ago by crumbking (previous) (diff)

comment:9 Changed 6 years ago by shevek

All my fault obviously. Commit 62f9b676 had some changes to hide group only post built in that I wasn't aware of anymore.

While being at the wrong place to really hide the post from the list there's obviously something wrong as well.

comment:10 in reply to: ↑ 7 Changed 6 years ago by shevek

Replying to crumbking:

I tested locally. Found a strange bug now. On one of your groups start a group only thread. Hit the reply button and the text of all "group only" posts below the reply form are not visible.

I checked that behaviour on www. If you reply to a topic in a group that you're not a member of the list doesn't contain the group only posts. Makes sense somehow as you probably won't reply to them ;-)

Last edited 6 years ago by shevek (previous) (diff)

comment:11 Changed 6 years ago by shevek

An interesting observation: The default for posts to groups is 'GroupOnly?' which works fine as long as you're a member of that group. If not you post and you can't read your post any longer.

That relates to #1814. If it wouldn't be possible to reply inthe first place the problem wouldn't exist.

Question is now: Fix the default for replies of non members? Or fix #1814 instead?

comment:12 follow-up: Changed 6 years ago by mahouni

I would say fix 1814. Question then would be: For 1.4 release or later?

I am pragmatic and don't mind if it is in a later release. But default is default, so we will certainly have some members wondering. Someone has to tell them to join the group to see their posts... Other question: Do people agree that you could reply only if you are a group member?

I tested a little bit, and it seems to work very well. Knowing about the complexity of that stuff, I am astonished. Great work! I guess it is difficult to test everything, and we might find some nice surprises when on alpha and www later. But I think it is worth to try.

Anything else that has to be tested locally explicit for this ticket here?

comment:13 in reply to: ↑ 12 Changed 6 years ago by shevek

  • Status changed from local_testing to to_alpha

I would say fix 1814. Question then would be: For 1.4 release or later?

Later. That would need checks on the forum display on the direct reply link etc.

But we can still ease the situation for 1.4. As the group to which the post belongs is known I can just remove the group only visibility from the list for none member.

But default is default, so we will certainly have some members wondering. Someone has to tell them to join the group to see their posts...

We do that already. Checked in as part of #1882. So if you posted as 'Group Only' accidentally you at least are told that you can't see your post as you're not a member of the group (if you're not a member of cheap travels you can see this here: http://alpha.bewelcome.org/forums/s4815-Cheap_Train_Travel_in_Austria_and_Germany___Group_Tickets)

I tested a little bit, and it seems to work very well. Knowing about the complexity of that stuff, I am astonished. Great work!

Thanks, took some iterations.

Anything else that has to be tested locally explicit for this ticket here?

Actually the code is already on alpha. So switch to testing.

comment:14 Changed 6 years ago by shevek

  • Status changed from to_alpha to testing

comment:15 Changed 6 years ago by shevek

Adding three lines to the forum model (forums.model.php)

 public function GetGroupEntity($IdGroup) {
        return $this->createEntity('Group')->findById($IdGroup);
    }

and replacing the part in the if branch starting at line 448 in forums.view.php with

            // getting group entity from model as createEntity isn't public
            $group = $this->_model->GetGroupEntity($IdGroup);
            $groupOnly = ($group->VisiblePosts == "no");
            $currentVisibility = "GroupOnly";
            if ($groupOnly) {
                $visibilities[] = "GroupOnly";
            } else {
                $visibilities[] = "NoRestriction";
                $visibilities[] = "MembersOnly";
                if ($group->isMember($this->_model->getLoggedInMember())) {
                    $visibilities[] = "GroupOnly";
                } else {
                    // No GroupOnly if no member so switch current visibility
                    $currentVisibility = "MembersOnly";
                }
            }

fixes the visibility. Adding the getGroupEntity function also helps to get rid of a SQL query. Could someone test these last changes locally?

comment:16 Changed 6 years ago by shevek

  • Status changed from testing to needs_work

comment:17 Changed 6 years ago by shevek

  • Status changed from needs_work to local_testing

comment:18 Changed 6 years ago by crumbking

Ähh what to test locally? Short hint how it was before and it should be now?

comment:19 follow-up: Changed 6 years ago by shevek

Arggh. Wrong ticket...

If you reply to a group post and you're not a member it shouldn't show 'Group Only' anymore. Allowing you to only post as members only or world.

Last edited 6 years ago by shevek (previous) (diff)

comment:20 in reply to: ↑ 19 Changed 6 years ago by mahouni

  • Status changed from local_testing to to_alpha

Replying to shevek:

If you reply to a group post and your not a member it shouldn't show 'Group Only' anymore. Allowing you to only post as members only or world.

reviewed and tested locally

comment:21 Changed 6 years ago by jsfan

  • Status changed from to_alpha to testing

comment:22 Changed 6 years ago by jsfan

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

Tested and works.

comment:23 Changed 6 years ago by jsfan

Oops. Maybe someone else can confirm. I think part of this looks different for me as a forum moderator. :(

comment:24 Changed 6 years ago by shevek

Actually it shouldn't as Mods only change the ThreadVisibility...

Last edited 6 years ago by shevek (previous) (diff)

comment:25 Changed 6 years ago by jsfan

Well, I tried a number of threads and all of them only gave me "Group only" in the drop down. But found one now which gave me other options. :)

Last edited 6 years ago by jsfan (previous) (diff)

comment:27 Changed 6 years ago by jsfan

I can only select "members only" there. Is that as it should be?

comment:28 Changed 6 years ago by shevek

Yes. ppec started the thread with visibility members only. As you could only select a lower visibility you're stuck with 'members only' as it is a forum thread and not a group thread.

comment:29 Changed 6 years ago by shevek

  • Resolution fixed deleted
  • Status changed from closed to reopened

As sitatara pointed out in comment 19 in #1838 currently replies to group always default to groups.

The last fix was a bit too aggressive. Here a less aggressive one: https://gitorious.org/bewelcome/rox/commit/9d95da5fc54081a7e53f8b1279ba38b57ce891e4

comment:30 Changed 6 years ago by shevek

  • Status changed from reopened to local_testing

This will keep the current visibility of the thread in line with the thread visibility. If you're not a member of the group and the group's visibility setting is group your reply will be group only!

Last edited 6 years ago by shevek (previous) (diff)

comment:31 Changed 6 years ago by mahouni

  • Status changed from local_testing to to_alpha

tested

comment:32 Changed 6 years ago by shevek

Just one update to allow replying to non group posts that you saw and keeping them visible for you: https://gitorious.org/bewelcome/rox/commit/843801a2cab1e8f9941f75b9c82cc651ce601603

comment:33 Changed 6 years ago by mahouni

  • Status changed from to_alpha to testing

tested and works too.

comment:34 Changed 6 years ago by mahouni

  • Status changed from testing to needs_work

sorry, I changed ticket status to deployed on alpha, but it isn't yet. I meant local test was successful, and ready for alpha.

Last edited 6 years ago by mahouni (previous) (diff)

comment:35 Changed 6 years ago by mahouni

  • Status changed from needs_work to to_alpha

comment:36 Changed 6 years ago by jsfan

  • Status changed from to_alpha to testing

comment:37 Changed 6 years ago by mahouni

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

tested on alpha. works fine.

Note: See TracTickets for help on using tickets.