Only NO_SERVICE disruptions on line_sections are linked on /lines requests

eturck commented 7 years ago

Hello!

We are using a lot of disruptions on line sections. For example it is used for :

A deviation, there is a NO_SERVICE impact on a section,
An elevator out of service, there is an OTHER_EFFECT impact on a section of the subway line on only one stop,
A stop is temporarily moved a few meter because of road work, there is a STOP_MOVED impact on a line section,
There is slight delays or minor problems on a section of line, but buses are still passing, we have another OTHER_EFFECT impact.

The problem we have is, when we are requesting /v1/coverage/tisseo/lines, some disruptions are not shown. And that's all the disruptions which are not with a "NO_SERVICE" effect.

We discuss this a bit on the phone with @kinnou02 and @antoine-de a few months ago, but we never made the issue. The problem appeared when you changed a bunch of stuff to be able to properly display disruptions on line_section, which is a good thing, but introduced this :).

I kind of know why it is happening, but don't really know how to properly fix it. It's in pb_converter : https://github.com/CanalTP/navitia/blob/dev/source/type/pb_converter.cpp#L620 And there is a pretty obvious comment about it. Here is the relevant part :

if (dump_message_options ==  DumpMessageOptions{DumpMessage::Yes, DumpLineSectionMessage::Yes} ) {
    /*
     * Here we dump the impacts which impact LineSection.
     * We could have link the LineSection impact with the line, but that would change the code and
     * the behavior too much.
     * */
    std::set<boost::shared_ptr<nt::disruption::Impact>> added_impact;
    auto fill_line_section_message = [&](const nt::VehicleJourney& vj) {
        for(const auto& impact: vj.meta_vj->impacted_by) {
            auto impact_ptr = impact.lock();
            if (! impact_ptr)
                continue;
            for (const auto& entity: impact_ptr->informed_entities()) {
                if (boost::get<nt::disruption::LineSection>(&entity) &&
                        added_impact.insert(impact_ptr).second ) {
                    fill_messages(vj.meta_vj, line);
                    return true;
                }
            }
        }
        return true;
    };
    for(const auto* route: l->route_list) {
        route->for_each_vehicle_journey(fill_line_section_message);
    }
}

Problem is, we loop on the vehicle_journeys to see if they are impacted, and we add the message to the line if that's the case. But in apply_disruption we have, at the start of the method applying a line section disruption :

if (impact->severity->effect != nt::disruption::Effect::NO_SERVICE) {
    LOG4CPLUS_DEBUG(log, "Unhandled action on " << uri);
    this->log_end_action(uri);
    return
}

The OTHER_EFFECT disruptions will not impact any vehicle_journey, therefore, we don't have it in /lines. Without too much thinking I can see two ways to fix this, but I really don't know the side effect they would cause :

Linking the line_section disruption to the line, but with @antoine-de's comment I am afraid it is a bad idea :),
Maybe still impacting the meta_vj but without creating an adapted vj? Would this break something/everything?

Let me know what you think!