Skip to content
This repository has been archived by the owner on Feb 1, 2024. It is now read-only.

Correct Facility Parent Company link #772

Merged
merged 2 commits into from Sep 10, 2019
Merged

Conversation

rajadain
Copy link
Contributor

@rajadain rajadain commented Sep 6, 2019

Overview

There is no detail page for Facility Parent Company entities, as both Claim Contributors and Facility Parent Company links open the same page. In cases when the IDs of the two are not in sync, clicking the Facility Parent Company link would go to the wrong page.

By making the link always point to the Claim Contributor's id, we ensure that it is always correct.

Connects #729

Demo

2019-09-06 11 19 54

Testing Instructions

  • Follow the instructions in the issue to simulate a situation where the user id and parent company id are out of sync
  • Submit a claim
  • Login as c1@example.com and go to the dashboard to view its details
  • Click the Claim Contributor link. Ensure it works.
  • Click the Facility Parent Company link. Ensure it works.

Checklist

  • fixup! commits have been squashed
  • CI passes after rebase
  • CHANGELOG.md updated with summary of features or fixes, following Keep a Changelog guidelines

@kellyi
Copy link
Contributor

kellyi commented Sep 9, 2019

Testing this out and I don't think it's doing the correct thing:

For this link, it should go to the contributor page for "Civil Society Organization G" but instead it is going to "Factory A":

Screen Shot 2019-09-09 at 1 34 49 PM

@kellyi
Copy link
Contributor

kellyi commented Sep 9, 2019

Chatted about this and it seems like we need to update the serializer to return the correct id value. We'll also need to update https://github.com/open-apparel-registry/open-apparel-registry/blob/develop/src/app/src/components/FacilityDetailSidebarClaimedInfo.jsx#L85 to match whatever changes we decide to make to the serializer and the dashboard claim details page.

@rajadain
Copy link
Contributor Author

rajadain commented Sep 9, 2019

I reverted the original front-end implementation and took another look at the serializer. Turns out we were inconsistent about the parent company's id, so I standardized to using the company's admin's id. This fixes the link issue on the front-end as well. Ready for another look.

Copy link
Contributor

@kellyi kellyi left a comment

Choose a reason for hiding this comment

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

👍 Tested this out by going through the steps to recreate the bug outlined in #729 and the links are now pointing to the correct contributor page!

@kellyi
Copy link
Contributor

kellyi commented Sep 9, 2019

Heads up that there's a merge conflict in the CHANGELOG file again.

@kellyi kellyi assigned rajadain and unassigned kellyi Sep 9, 2019
@kellyi
Copy link
Contributor

kellyi commented Sep 10, 2019

This has another merge conflict.

Previously we would sometimes use the parent contributor's id,
and sometimes the parent contributor's admin's id. Since we use
the latter to view profiles on the front-end, standardize to it
in all cases.
@rajadain
Copy link
Contributor Author

That should fix it.

@rajadain
Copy link
Contributor Author

Thanks for taking a look, and for talking me through this. Will merge when green.

@rajadain rajadain merged commit 84a11f8 into develop Sep 10, 2019
@rajadain rajadain deleted the tt/fix-parent-company-link branch September 10, 2019 16:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants