Signal-Cartel / EveScoutRescue

Web site and data tools for evescoutrescue.com, maintainer ThriceHappy.
https://evescoutrescue.com/home/
13 stars 10 forks source link

Fix for issue #217 #222

Closed Klensor closed 5 years ago

Klensor commented 5 years ago

For the tend, I'm also checking for upkeep required status because I figured we wanted a record of when caches are set to Upkeep required in any case (as those are not shown in the log). Let me know if this isn't correct.

Thricehappy commented 5 years ago

@chigaze Before I merge and push to production, can you also comment on this? Thanks!

Klensor commented 5 years ago

It feels to me like something doesn't match up quite right - why don't we record 'upkeep required' in the action list?

As I'm new to all this I don't know if it was a deliberate choice. If it was, then my suggested fix doesn't seem to sit right with that. If it was an oversight, then perhaps we should also add 'upkeep required' events in the action log?

chigaze commented 5 years ago

Note on my answers: Right now I only know the system from the UI end so apologies for anything I misinterpret. I’ll get up to speed on the back end structure over the next few weeks.

For this one I would say Agent actions show up in the history and they, ASFAIK, always mark the cache as needing upkeep. That said, I think you’re referring to the case where on a regular tend the cache is marked as needing upkeep, as would happen if someone tended, realized the cache was empty, but had no supplies. Something in the History that indicated that would be good.

Also I presume when you say Action, you’re referring to the line items that show up in the History.

bikerunner commented 5 years ago

I checked the current behavior. The history type distinguish between Sower and Tender. But Tender contains the (internal) status "Healthy" and "Upkeep required".

Would it be a solution to:

Klensor commented 5 years ago

Thanks @chigaze and @bikerunner for your feedback. I will try and implement as you've both outlined.

I hadn't realised that Agent actions would also set the cache to "Requires Update", although that makes perfect sense. That does lead me to one more question for @chigaze though... If I create a SAR, then set the status to "Closed - rescued (ESRC)" this DOES NOT set the cache to "Requires Upkeep". Is this a bug? For now I'm assuming so, and will try and fix that here also...

bikerunner commented 5 years ago

@Klensor This is an idea from me, how this can work. @chigaze should approve it as master of ESRC.

The "Upkeep" status after an ESRC rescue depends on the pilot. Some pilots return the supplies and some not. So the coordinator should set it manually depending on the pilots response I think.

Klensor commented 5 years ago

I've updated all the things as per the discussion. I have not modified the way SARs work, I'll leave that for a future since coordinators can modify the status as per @bikerunner's suggestion.

There is a new column to keep track of cache status values on each action: ALTER TABLE activity ADD CacheStatus VARCHAR(25) NULL DEFAULT NULL AFTER IP;

I've changed quite a few things - hopefully I haven't broken anything major, and hopefully the change to the history isn't too eyewatering (I'm no CSS wiz!)

Thricehappy commented 5 years ago

@bikerunner How do you want to test PRs like this? Let me know what process works for you. I can merge branch and push to production once I get the OK from Engineering (i.e., you all). :-)

chigaze commented 5 years ago

Thanks Klensor, those changes sound good. Ideally, in the long run, it would be good to only have a single interface for all rescues, with the various options, rather than separate "Agent" and "SAR" buttons.

bikerunner: While pilots do often return the probes our practice is to always set it to Upkeep required with a note indicating the pilot said they returned them. We can never be absolutely sure they were returned until we look for ourselves.

Klensor commented 5 years ago

Removing the notes entirely is not ideal as there might be some circumstances where the notes would not otherwise be visible (e.g. when cache details are modified by admin), so I've added them back so they are hidden by default but toggleable.

bikerunner commented 5 years ago

@Klensor Can you check if this issue relates to #128 ?

chigaze commented 5 years ago

Looking very good. As per Discord, widen the colour on the left to 6px. Also, would it be possible to move the System Notes buttons to either above the History or above the current cache data to increase its visibility.

image
bikerunner commented 5 years ago

From code style I can approve the changes.