Skip to content

Comments

Load project as needed#2643

Merged
jake-low merged 1 commit intomaproulette:mainfrom
jlewin:issue-2642
May 27, 2025
Merged

Load project as needed#2643
jake-low merged 1 commit intomaproulette:mainfrom
jlewin:issue-2642

Conversation

@jlewin
Copy link
Contributor

@jlewin jlewin commented May 24, 2025

@jlewin
Copy link
Contributor Author

jlewin commented May 24, 2025

This is my take on a fix and completely understandable if it varies from a more preferred approach. If nothing else, hopefully it provides a glanceable take on the problem and helps bring about a fix.

  1. Small changes were made in mapStateToProps to ensure the loaded identifiers remain available for subsequent processing
  2. The retained projectId is passed into and used in loadTask to ensure the project data is available and mapped into redux state
  3. References in the form of existingTask?.parent?.parent are extracted to named variables for clarity in a somewhat hard to track scope of behavior.
  4. Rather than build up a complex and opaque object to pass to fetchParentProject, fetchProject is imported and used to directly to load the project with only the known identifier

This worked but seemed less ideal...

 const challenges = {};
  challenges[projectId.toString()] = {
    parent: projectId
  };
  const mockChallengeResults = {
    entities: {
      challenges: challenges,
    },
    result: projectId,
  };
  fetchParentProject(dispatch, mockChallengeResults);

versus

dispatch(fetchProject(projectId));

@jake-low
Copy link
Contributor

Awesome, thank you so much for diving into this bug and putting together a PR! I'll review this after the weekend but just wanted to say thanks for contributing.

Copy link
Contributor

@jake-low jake-low left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! This logic for loading tasks was tricky; thanks for figuring out the bug. In the future it'd be good for us to simplify this logic so it's less error prone.

@jake-low jake-low merged commit d101399 into maproulette:main May 27, 2025
@jlewin jlewin deleted the issue-2642 branch May 27, 2025 22:20
@CollinBeczak CollinBeczak mentioned this pull request Jun 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Challenge get-by-id endpoint results in indefinite loading indicator

2 participants