Open-event-server: `before_delete_object` not being called in orders.py

[prateekj117]

Describe the bug
As we are not deleting the order and just updating the deleted_at field, before_delete_object is not being called.

Additional context
Working on it.

[itnug987]

Is this issue still open? Can I work on this issue??

[prateekj117]

@itnug987 Go ahead.

[akshat0047]

I would like to work on this issue @prateekj117

[prateekj117]

@akshat0047 Go ahead.

[Tlazypanda]

@prateekj117 can I take this up?

[prateekj117]

@akshat0047 Are you still on it?

[akshat0047]

This issue/feature has been discussed as not required in Gitter discussions, maybe this needs to be closed @prateekj117 @iamareebjamal

[prateekj117]

The discussion says we don't want the order deletion functionality altogether, then we need to implement that, otherwise right now, because of this issue, anyone can delete anyone's order and we can also delete a placed/deleted order. @iamareebjamal

[akshat0047]

So should I continue with the fix why the order is not being deleted, as there was a bug occurring while fixing the issue which leads to this discussion or a new issue referencing this one is to be worked upon?

[Tlazypanda]

@prateekj117 Is this issue still available?

[prateekj117]

Need your insight @iamareebjamal.

[iamareebjamal]

This should be closed and functionality of deleting an order should be removed

[kushthedude]

This should be closed and functionality of deleting an order should be removed

@iamareebjamal As I told before in gitter chat, this functionality should be available to Admin/Super Admin, As when creating orders, some orders are expired and some orders are set to initialising, Also we have a corresponding issue for implementing a orders tab for Admin Dashboard where he can see the deleted orders too, That is why we removed the delete orders tab from the event dashboard.
Also currently, the order creation proccess is bit buggy. As some dummy orders with label initialising gets created and it does not get remove after the order is expired. I suggest to keep the functionality untill the order creation and labelling is completly bug free.
https://github.com/fossasia/open-event-frontend/pull/3226
https://github.com/fossasia/open-event-frontend/issues/3168

[prateekj117]

@iamareebjamal As I told before in gitter chat, this functionality should be available to Admin/Super Admin, As when creating orders, some orders are expired and some orders are set to initialising.

If some orders are expired and some orders are set to initializing, how does it imply that this functionality should be available to Admin/Super Admin.

As some dummy orders with label initialising gets created and it does not get remove after the order is expired.

If this issue was known, why wasn't it reported on the server?
And are you sure it happens on the server and the status column of orders does not update to expired?

[kushthedude]

Play with web app after creating more than 20 orders for each event. You
will see for yourself.

On Sun, 3 Nov, 2019, 02:56 Prateek Jain, [e-mail] wrote:

@iamareebjamal https://github.com/iamareebjamal As I told before in
gitter chat, this functionality should be available to Admin/Super Admin,
As when creating orders, some orders are expired and some orders are set to
initialising.

If some orders are expired and some orders are set to initializing, how
does it imply that this functionality should be available to Admin/Super
Admin.

As some dummy orders with label initialising gets created and it does not
get remove after the order is expired.

If this issue was known, why wasn't it reported on the server?
And are you sure it happens on the server and the status column of orders
does not update to expired?

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/fossasia/open-event-server/issues/6450?email_source=notifications&email_token=AKQMTLSSEICMJRVKR2LVR6TQRXWBLA5CNFSM4IXUAQSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEC5FMKA#issuecomment-549082664,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AKQMTLXELE3ASWBO3VKWXD3QRXWBLANCNFSM4IXUAQSA
.

[kushthedude]

There is no sense in showing dummy orders and initialising orders to the
event dashboard, also see on FE some of the placed orders appears as pay
with paypal.
Hence its necessary to delete ambiguos orders.
You must be knowing for yourself eventyay is not completely bug free till
now. Until we get to a more stable system, I suggest to retain the
functionality of Deletion.

On Sun, 3 Nov, 2019, 03:11 Kush Trivedi, [e-mail] wrote:

Play with web app after creating more than 20 orders for each event. You
will see for yourself.

On Sun, 3 Nov, 2019, 02:56 Prateek Jain, [e-mail] wrote:

@iamareebjamal https://github.com/iamareebjamal As I told before in
gitter chat, this functionality should be available to Admin/Super Admin,
As when creating orders, some orders are expired and some orders are set to
initialising.

If some orders are expired and some orders are set to initializing, how
does it imply that this functionality should be available to Admin/Super
Admin.

As some dummy orders with label initialising gets created and it does not
get remove after the order is expired.

If this issue was known, why wasn't it reported on the server?
And are you sure it happens on the server and the status column of orders
does not update to expired?

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/fossasia/open-event-server/issues/6450?email_source=notifications&email_token=AKQMTLSSEICMJRVKR2LVR6TQRXWBLA5CNFSM4IXUAQSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEC5FMKA#issuecomment-549082664,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AKQMTLXELE3ASWBO3VKWXD3QRXWBLANCNFSM4IXUAQSA
.

[iamareebjamal]

Deletion should not be possible. Even if there is dummy data, the root cause should be fixed and the dummy data should be removed.

[kushthedude]

Regarding placed order with payment:
https://github.com/fossasia/open-event-frontend/issues/3505

Also we dont have a good amount of testing for FE ,So it gets difficult to check if the error is from web client or server.

[prateekj117]

Play with web app after creating more than 20 orders for each event. You will see for yourself.

By your tone, you are sure it is a server-side issue, right? And that the order status is not changing. Good enough then, please open an issue and I will have a look into it.

There is no sense in showing dummy orders and initialising orders to the
event dashboard, also see on FE some of the placed orders appears as pay
with paypal.
Hence its necessary to delete ambiguos orders.

Let's delete everything which goes wrong from now on?

[kushthedude]

Play with web app after creating more than 20 orders for each event. You will see for yourself.

By your tone, you are sure it is a server-side issue, right? And that the order status is not changing. Good enough then, please open an issue and I will have a look into it.

There is no sense in showing dummy orders and initialising orders to the
event dashboard, also see on FE some of the placed orders appears as pay
with paypal.
Hence its necessary to delete ambiguos orders.

Let's delete everything which goes wrong from now on?

If its the case , you can go and become the one man army and solve the bugs for yourself. Also, if you have tested and played with the web app more, you might have got the GIST of each and everything I am talking about. But since you are not in attitude of listening to me.
Go on and proceed as you wish.
Thanks

[kushthedude]

@iamareebjamal I don't need to mention how the crucial server is to web-client and vice-versa.
You have a much greater knowledge about the functioning of Server then I have. But I know a bit more about the Web-Client and the current issues. I know the frontend has a quite amount of bugs and out of which some are happening only in productions.

The issue links I mentioned above and the points I stated, it is clear from them we need to provide a Delete Order functionality which can be managed by an admin until we have a more stable.
But even after, if delete order features obsolete to you, I have no objections.

But I don't expect any invalid or heated arguments from any other except the maintainer and mentor of the project, because it has been the fourth time @prateekj117 has directly raised an argument with me rather providing actual intellect or issue references. And I don't need to mention my contributions to the Web Client and FOSSASIA as an organisation.

[iamareebjamal]

I see no one raising an argument here except you

By your tone, you are sure it is a server-side issue, right? And that the order status is not changing. Good enough then, please open an issue and I will have a look into it.

This is asking if it is a server issue or frontend. And to fix it.

Which I agree with. If you think delete orders should be kept, then give the reasons why? Because of bugs in frontend? Why can't they be fixed?

Deleting is an extremely dangerous action, specially for something involving real life value - money in this case. A month back, a normal speaker could get access to the admin panel. The argument that delete order functionality should be kept because system is buggy serves opposite to its purpose. It should be removed because the system is buggy

And about knowledge of server or frontend, no, I have less knowledge of both than you and Prateek. Just more knowledge of what should happen, and not what is happening.

My goal here is to take the system to that level of should happening, not to implement workarounds to keep the current buggy system working.

Leaving a disastrous functionality in because removing it will break stuff leads to what we call "legacy code". This is not the correct approach. Whatever will and can be broken will be fixed before the PR of removing the deletion of orders is merged. Why do you think https://github.com/fossasia/open-event-server/pull/6555 and https://github.com/fossasia/open-event-server/pull/6545 are not merged yet?
I haven't confirmed that they will not break stuff.

I am the quality control. Something breaks in production and it is a PR I merged, gets blamed on me, doesn't matter if you, Prateek or Guido van Rossum wrote it. So, I take the quality and correctness of things more seriously than you guys, so rest assured.

[iamareebjamal]

Also, what is this elitist attitude of directly raised an argument against me?

This is not military where there is a chain of command. This is an open-source project. This is a democracy, everyone is supposed to raise arguments against anyone. You have done it many times to me. Did I flip my arms out and started bickering like kids like you 2 are.

Everyone is allowed to criticize any idea and raise his/her concerns with constructive criticism