Arakne / Araknemu

Simple Dofus server in Java
GNU Lesser General Public License v3.0
109 stars 28 forks source link

feat(player): :sparkles: add soft delete players #310

Closed Jean-Dv closed 10 months ago

Jean-Dv commented 1 year ago

Hi ! Looking at the issues a little, I decided to implement soft delete of players.

Implementation

Usually a field is used in the database to help the system check whether it is active or not. It is for this reason that I add it and also do some validations for when the data is sent to the client.

Problems

When running the checkstyle I get the following error: image

I understand the error, however, I have no idea how to fix it in a "clean" way.

vincent4vx commented 1 year ago

Hi! Thanks for the pull request. So, for the issue on checkstyle is to add a ignore rule on checkstyle.xml :


    <module name="SuppressionSingleFilter">
        <property name="checks" value="MethodCountCheck" />
        <property name="files" value="entity.player.Player" />
    </module>

but in this particular implementation, the field and accessors for "is active" flag are not necessary and can be removed, so the methods count will drop under the limit. And to test this you can add a functional test which creates then delete, then get to ensure that the entity is no more accessible, with an addition of a low level test which call manually the database to check the value of column IS_ACTIVE.

I have two suggestion to improve this feature, if you want/have time to do this :

vincent4vx commented 10 months ago

Hi!

I would like to release the v0.11 shortly, therefore I want to merge this PR. Do you have the time to write requested tests, or do you prefer that I do it myself ?

Thanks, and happy holidays !

Jean-Dv commented 10 months ago

Yes, if you prefer you can modify my PR :D

vincent4vx commented 10 months ago

Yes, if you prefer you can modify my PR :D

Ok, thanks ! I'll do that after fixes last reported bugs :wink:

vincent4vx commented 10 months ago

The pull requested is continued here #320 So I close this one. Thanks for the contribution !