Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ban Module #220

Merged
merged 3 commits into from
Jul 21, 2019
Merged

Ban Module #220

merged 3 commits into from
Jul 21, 2019

Conversation

veryard
Copy link
Contributor

@veryard veryard commented Mar 22, 2019

  • phpBB 3 Ban Module

- phpBB 3 Ban Module
Copy link
Member

@euantorano euantorano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, just some minor comments that I've spotted so far.

I haven't tested this at all yet, to do so I will need to spin up a PHPBB 3 install and ban some users. I'll try and get a chance to do that this weekend.

boards/phpbb3.php Show resolved Hide resolved
resources/modules/bans.php Outdated Show resolved Hide resolved
boards/phpbb3/bans.php Outdated Show resolved Hide resolved
@burner1024
Copy link

Just tried this.

  1. All bans end today (well, at least permanent ones, I only have those).
  2. Ban reason is missing.
  3. Ban source is always "MyBB Engine".
  4. Several "Guest" accounts somehow creeped in? Maybe those are deleted users?

Captura de pantalla de 2019-03-22 21-20-09

I don't mean to be picky, of course, just trying to help too.

@veryard
Copy link
Contributor Author

veryard commented Mar 22, 2019

Just tried this.

  1. All bans end today (well, at least permanent ones, I only have those).
  2. Ban reason is missing.
  3. Ban source is always "MyBB Engine".
  4. Several "Guest" accounts somehow creeped in? Maybe those are deleted users?

Captura de pantalla de 2019-03-22 21-20-09

I don't mean to be picky, of course, just trying to help too.

Thanks for the feedback:

  1. Will have a look, MyBB has a weird dateline field that i'll need to code from phpBB.
  2. phpBB has two reasons, one that is given and one that is shown, at the moment i'm just pulling the ban_reason not the ban_give_reason (I'll make it pull the ban_give_reason if the initial one is empty)
  3. I don't think phpBB store the user id against the ban when banned? (Would need to pull from the phpbb_log but these can be cleared so im not to sure.
  4. I'm not sure about this, I only had 2 users and they both came through no guests.

I'll get to everything on Sunday.

Thanks,
Brad.

@euantorano
Copy link
Member

I'll keep this as requiring changes until I've had a chance to test, then approve it if it looks like everything works. Thanks for the contribution 😄

@burner1024
Copy link

burner1024 commented Mar 22, 2019

  1. I definitely have an account with both reasons filled in, still comes up empty in MyBB.
  2. I think you're right about the logs. It would be helpful to pull the source from there, but it's not crucial.
  3. Yes, it appears that deleted users can leave lingering bans, db integrity is not enforced. Can probably just skip these:
select count(ban_id) from phpbb_banlist where ban_userid not in (select user_id from phpbb_users);
+---------------+
| count(ban_id) |
+---------------+
|             8 |
+---------------+
  1. Also, there are IP and email bans, ideally they should be transferred as well.

- Made reason pull from give reason if initial reason is empty.
- GID defaults to 7
- Ban Time added
- No longer pulls deleted or users that do not exist.
@veryard
Copy link
Contributor Author

veryard commented Mar 23, 2019

Hello,

Have made changes:

  • Reason will pull from ban_give_reason if ban_reason is empty
  • Deleted users or users that do not exist in the users table are ignored.
  • bantime is now generated based from the phpBB fields ban_start & ban_end, treats bans less than one day as a one day ban. (phpBB allows users to be banned for 30 mins, 1 hour etc etc.)
  1. Also, there are IP and email bans, ideally they should be transferred as well.

I looked into this, looks like MyBB stores these bans in the banfilters table which is a bit odd considering it all could be done from one table. I'll have to extend the code to do this some where down the line.

@euantorano
Copy link
Member

euantorano commented Mar 23, 2019 via email

@veryard
Copy link
Contributor Author

veryard commented Mar 24, 2019

Let me know if you need me to fix/update anything.

Brad.

@burner1024
Copy link

What if the text contains valid tags, though? Code snippets, etc.

@veryard
Copy link
Contributor Author

veryard commented Mar 25, 2019

What if the text contains valid tags, though? Code snippets, etc.

Sorry, I don't understand what you're saying?

@burner1024
Copy link

err... that was for the pull for issue #223

@veryard
Copy link
Contributor Author

veryard commented Mar 25, 2019

err... that was for the pull for issue #223
I see, well phpBB uses BBCodes aswell, so they are code snippets are inside the [code] BBCode.

@euantorano euantorano merged commit 85a5c9d into mybb:feature Jul 21, 2019
@JordanMussi JordanMussi mentioned this pull request Jun 16, 2020
This was linked to issues Jun 16, 2020
@JordanMussi JordanMussi removed a link to an issue Jun 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bans aren't transferred from PHPBB3
3 participants