Problem/Motivation

An entity that already references a Mapillary Sequence through the "Mapillary Sequence ID" widget (mapillary_sequence_id, the default widget of mapillary_sequence_reference) cannot be saved again. The form fails with "Could not import Mapillary Sequence 1: ...", where 1 is not a Mapillary ID.

The widget mixes the two IDs of a sequence:

  • Mapillary Sequence IDs are alphanumeric, so mapillary_sequence keeps them in the sequence_id base field. The entity ID is an ordinary auto-increment integer, and that is what the reference field stores in target_id.
  • MapillarySequenceIdWidget::formElement() fills the text field with target_id, so the editor sees the local entity ID.
  • MapillarySequenceIdWidget::massageFormValues() passes the submitted value to MapillaryImageImporter::importSequence(), which looks it up as a Mapillary Sequence ID (loadSequenceByNaturalId()). No sequence has that ID, so the importer asks the Graph API for /image_ids?sequence_id=1, gets no images (or an error), and throws. The widget turns the exception into a form error.

The other ID widgets are not affected: mapillary_image and mapillary_map_feature use the Mapillary ID as the entity ID. The Leaflet sequence widget already handles this: MapillaryLeafletSequenceWidget::getMapillaryId() reads sequence_id from the referenced entity. MapillarySequenceIdWidget has no test.

Steps to reproduce

  1. Enable mapillary_field and set an access token at /admin/config/services/mapillary/api.
  2. Add a "Mapillary Sequence Reference" field to the Article content type and keep the default "Mapillary Sequence ID" widget.
  3. Create an article, enter a Mapillary Sequence ID and save. The sequence is imported and the article is saved.
  4. Edit the article. The field shows a small integer (the local entity ID), not the Mapillary Sequence ID that was entered.
  5. Save without changing anything. The form shows "Could not import Mapillary Sequence 1: ..." and the article is not saved.

Proposed resolution

  • In MapillarySequenceIdWidget::formElement(), take the default value from the referenced entity's sequence_id, as MapillaryLeafletSequenceWidget does. A reference whose target no longer exists shows an empty field, as in the Leaflet widget.
  • No change to massageFormValues(): with the Mapillary ID in the field, importSequence() finds the local sequence and makes no API request.

Out of scope: the import loop that MapillaryImageIdWidget, MapillarySequenceIdWidget, MapillaryMapFeatureIdWidget and MapillaryLeafletWidgetBase each implement. A follow-up issue will share it.

Remaining tasks

  • A functional test, along the lines of MapillaryImageIdWidgetFunctionalTest: with a local sequence and a node referencing it, the edit form shows the Mapillary Sequence ID, and saving it unchanged keeps the reference without an API request. The test fails before the fix.
  • CHANGELOG.

User interface changes

  • The "Mapillary Sequence ID" field shows the Mapillary Sequence ID of an existing value instead of the local entity ID.

API changes

  • None.

Data model changes

  • None. Stored values do not change; sites need no update.

Issue fork mapillary-3629597

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

orkutmuratyilmaz created an issue. See original summary.

  • orkutmuratyilmaz committed 4ec6417c on 1.0.x
    bug #3629597: Sequence ID widget shows the local entity ID, so saving an...
orkutmuratyilmaz’s picture

Status: Active » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.