search

nabeelio / phpvms

virtual airline management
http://www.phpvms.net
BSD 3-Clause "New" or "Revised" License
172 stars 143 forks source link

Aircraft flight time is changed when rejecting a pending PIREP #1852

Closed arthurpar06 closed 1 month ago

arthurpar06 commented 3 months ago

Describe the bug When a PIREP is rejected, the reject method of the PirepService is called. This method decreases the aircraft's flight time by the flight time of the PIREP (see below). https://github.com/nabeelio/phpvms/blob/2f8a8dfb0e16f9a9ed242b0bd3f655f82c98a4a0/app/Services/PirepService.php#L702-L703

However, the aircraft's flight time is only incremented when the accept method of the PirepService is called (i.e., when the PIREP is accepted), as seen here: https://github.com/nabeelio/phpvms/blob/2f8a8dfb0e16f9a9ed242b0bd3f655f82c98a4a0/app/Services/PirepService.php#L663

This creates an issue when a PIREP is in a pending state (with auto-accept disabled). If the admin chooses to reject the PIREP, the flight time is deducted from the aircraft's total flight time, causing the aircraft's flight time to be less than it was before the PIREP was submitted.

Version Latest dev

To Reproduce Steps to reproduce the behavior:

  1. Go to the admin aircraft section and note the flight time of an aircraft.
  2. Submit a PIREP (either via ACARS or manually) with this aircraft.
  3. Reject the PIREP in the admin section.
  4. Observe the change in the aircraft's flight time.

Expected behavior The aircraft's flight time should not change if the PIREP is rejected since no accepted PIREPs were recorded for this aircraft.

Screenshots

Before: image

Pirep: image

After: image

FatihKoz commented 3 months ago

This remain hidden and not noticed because of nightly stats re-calculation probably... But unfortunately there is an open risk there too :wink:

That method does not check if the pirep is in pending, accepted or rejected state either and uses all pireps (including even cancelled ones)... So if an admin decides to keep all pireps in their db, not deleting them, this will not end up nice.

https://github.com/nabeelio/phpvms/blob/2f8a8dfb0e16f9a9ed242b0bd3f655f82c98a4a0/app/Services/AircraftService.php#L14-L24

I think we may need a PR for this method too, at least to include only ACCEPTED pireps.

What do you think ?