Merge application and project communication tabs #4349
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wes-otf
added
Type: Enhancement
frjo
requested changes
Jan 20, 2025
| {% comment %} Loaded using the htmx via alpine's custom event "open-tab-3"{% endcomment %} | ||
| <p>{% trans "Loading…" %}</p> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
We can delete this part from applicationsubmission_detail.html as well. I think we might even clear out all partial-activities urls and partial_submission_activities() unless they are still used somewhere else.
| @@ -208,31 +208,19 @@ <h5>{% trans "Project form approvals" %}</h5> | |||
| <div class="feed"> | |||
| {% if not object.is_archive %} | |||
| <h4 class="m-0 sr-only">{% trans "Add communication" %}</h4> | |||
| {% include "activity/include/comment_form.html" %} | |||
| {% include "activity/include/comment_form.html" with action=object.submission.get_absolute_url %} | |||
There was a problem hiding this comment.
This works but feels a bit, un neat. You go to projects/p:n/#communications, submit the form and end up on submissions/s:n/#communications. Not that normal users look at the address bar.
There was a problem hiding this comment.
This is a nice catch, I didn't even notice that was the case with the redirect. Do you think it should be handled in htmx so the redirect doesn't happen?
There was a problem hiding this comment.
Or does it make sense now to break out communications to its own view and url?
There was a problem hiding this comment.
that's a good idea, I can explore that a bit
There was a problem hiding this comment.
Another (more temporary) solution is to change up the get_success_url to resolve to the referrer URL:
So swapping
hypha/hypha/apply/activity/views.py
Lines 156 to 157 in b643557
with
def get_success_url(self):
if (ref := self.request.META.get("HTTP_REFERER")) and (path := urlparse(ref).path):
try:
url_resolved = resolve(path)
if url_resolved.url_name == "detail":
return f"{path}#communications"
except Resolver404:
# If it couldn't be resolved, use the default `get_absolute_url()`
pass
return f"{self.object.source.get_absolute_url()}#communications"There was a problem hiding this comment.
The redirect is not a big issue so I do not tink we need hack around it.
If creating a stand alone view/url is a bit big we can do it in a separate PR.
There was a problem hiding this comment.
Might be better to break the PRs up, I can take a stab at breaking it out into it's own view
theskumar
reviewed
Jan 22, 2025
frjo
approved these changes
Jan 22, 2025
frjo
added
Status: Needs testing
|
@wes-otf Trying to put this on test I get this error: |
|
Running it locally works it seems. |
theskumar
reviewed
Feb 2, 2025
Comment on lines
+53
to
+71
| proj_content_type = ContentType.objects.get_for_model(Project) | ||
| app_content_type = ContentType.objects.get_for_model(ApplicationSubmission) | ||
|
|
||
| # Determine if the provided object is an ApplicationSubmission or Project, then pull | ||
| # the related it's related Project/ApplicationSubmission activites if attribute it exists | ||
| if isinstance(obj, ApplicationSubmission): | ||
| source_filter = Q(source_object_id=obj.id, source_content_type=app_content_type) | ||
| if hasattr(obj, "project") and obj.project: | ||
| source_filter = source_filter | Q( | ||
| source_object_id=obj.project.id, source_content_type=proj_content_type | ||
| ) | ||
| else: | ||
| source_filter = Q( | ||
| source_object_id=obj.id, source_content_type=proj_content_type | ||
| ) | ||
| if hasattr(obj, "submission") and obj.submission: | ||
| source_filter = source_filter | Q( | ||
| source_object_id=obj.submission.id, source_content_type=app_content_type | ||
| ) |
There was a problem hiding this comment.
I believe this can further simplified like this,
logic to get the source filter.
if hasattr(obj, "project") and obj.project:
source_filter = Q(submission=obj) | Q(project=obj.project)
if hasattr(obj, "submission") and obj.submission:
source_filter = Q(submission=obj.submission) | Q(project=obj)
else:
source_filter = Q(submission=obj)
Suggested change
| proj_content_type = ContentType.objects.get_for_model(Project) | |
| app_content_type = ContentType.objects.get_for_model(ApplicationSubmission) | |
| # Determine if the provided object is an ApplicationSubmission or Project, then pull | |
| # the related it's related Project/ApplicationSubmission activites if attribute it exists | |
| if isinstance(obj, ApplicationSubmission): | |
| source_filter = Q(source_object_id=obj.id, source_content_type=app_content_type) | |
| if hasattr(obj, "project") and obj.project: | |
| source_filter = source_filter | Q( | |
| source_object_id=obj.project.id, source_content_type=proj_content_type | |
| ) | |
| else: | |
| source_filter = Q( | |
| source_object_id=obj.id, source_content_type=proj_content_type | |
| ) | |
| if hasattr(obj, "submission") and obj.submission: | |
| source_filter = source_filter | Q( | |
| source_object_id=obj.submission.id, source_content_type=app_content_type | |
| ) | |
| if hasattr(obj, "project") and obj.project: | |
| source_filter = Q(submission=obj) | Q(project=obj.project) | |
| if hasattr(obj, "submission") and obj.submission: | |
| source_filter = Q(submission=obj.submission) | Q(project=obj) | |
| else: | |
| source_filter = Q(submission=obj) | |
|
We'll also need to update the --- a/hypha/apply/activity/templatetags/activity_tags.py
+++ b/hypha/apply/activity/templatetags/activity_tags.py
@@ -84,7 +84,11 @@ def visibility_options(activity, user) -> str:
Returns:
A JSON string of visibility options
"""
- submission_partner_list = activity.source.partners.all()
+ if hasattr(activity.source, "partners"):
+ submission_partner_list = activity.source.partners.all()
+ else:
+ submission_partner_list = activity.source.submission.partners.all()
+
choices = activity.visibility_choices_for(user, submission_partner_list)
return json.dumps(choices) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #4249. Migrates all project comments to the associated application, and loads the applications comments via htmx (merging in project activities). This was the best solution I could come up with until we potentially take a step a coupling the two closer (ie. sharing an ID (#3944), a URL space, and/or a detail view).
I had tried another version of this that did the same migration but loaded the project view via htmx from the application detail view, which made everything feel more consistent, but I had concerns about the user needed to see the status/stages bar when making a comment which is where the UI got hairy.
Would love any suggestions/critiques!
Test Steps
Ensure...