Skip to content

Bug fix and cover a gap#601

Merged
rathod-b merged 3 commits intodevelopfrom
summary-endpoint-updates
Sep 25, 2024
Merged

Bug fix and cover a gap#601
rathod-b merged 3 commits intodevelopfrom
summary-endpoint-updates

Conversation

@rathod-b
Copy link
Copy Markdown
Collaborator

@rathod-b rathod-b commented Sep 24, 2024

Fix a few bugs and cover a gap in re-linking portfolios and runs.

Please check if the PR fulfills these requirements

  • CHANGELOG.md is updated
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce?

(Bug fix, feature, docs update, ...)
Bug fix

What is the current behavior?

(You can also link to an open issue here)
Runs cannot be unlinked; once a run is unlinked from a portfolio, it cannot be re-linked to that portfolio.

What is the new behavior (if this is a feature change)?

Runs can be unlinked and successfully relinked to portfolios

Does this PR introduce a breaking change?

(What changes might users need to make in their application due to this PR?)
No

Other information:

Fix a few bugs and cover a gap in re-linking portfolios and runs.
Comment thread reoptjl/views.py
for s in scenario:
s.portfolio_uuid = p_uuid
s.save()

Copy link
Copy Markdown
Collaborator Author

@rathod-b rathod-b Sep 24, 2024

Choose a reason for hiding this comment

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

@Bill-Becker Am I allowed to delete PortfolioUnlinkedRuns objects?

I realized that when we link a portfolio to an existing run, there can be a situation where the run is listed in PortfolioUnlinkedRuns, ie it was unlinked from a portfolio previously. In this case we do not want the run to show up in independent summary endpoint runs, which is why we'd have to delete that entry from PortfolioUnlinkedRuns table. I tested to see that an existing run can be deleted from PortfolioUnlinkedRuns so it doesnt show up in independent summary runs, and then unlinked from that portfolio so it does show up. This could be a niche use case so i can remove this.

Comment thread reoptjl/views.py
run_uuid__in=[i.run_uuid for i in UserUnlinkedRuns.objects.filter(user_uuid=user_uuid)]
).only(
'run_uuid',
'user_uuid',
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Added these two fields in Summary response for informational and validation purposes. The UI can ignore these.

Comment thread reoptjl/views.py
return JsonResponse({"Error": "Run {} is not associated with user {}".format(run_uuid, user_uuid)}, status=400)
else:
return JsonResponse({"Error": "Error in unlinking run {} from portfolio {}".format(run_uuid, portfolio_uuid)}, status=400)
pass
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed a bug here so unlinking actually happens.

Updated JSON response to delineate between adding a portfolio uuid vs deleting from existing portfolio runs.
@rathod-b rathod-b marked this pull request as ready for review September 25, 2024 20:37
@rathod-b rathod-b merged commit 0927396 into develop Sep 25, 2024
@rathod-b rathod-b deleted the summary-endpoint-updates branch September 25, 2024 21:05
@rathod-b rathod-b mentioned this pull request Sep 26, 2024
3 tasks
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.

2 participants