Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update public facing id of project to be the id of the submission #3944

Merged
merged 19 commits into from
Mar 11, 2025

Conversation

theskumar
Copy link
Member

Update the project urls to use it's submission id instead of the project's db id

The reason for the update is that there are two main entities in the system, one submission and other project.
The ids of the submission is linkable in the communiction and also used for referencing at different places.

Considering a project is an extension of application submission, if we have the project publically available id same as that of it's submission it always to link and reference the project by the same id.

Test Steps

  • General regression test and make sure all the urls in the project section resolve and link to the right pages

@theskumar theskumar self-assigned this May 28, 2024
@theskumar theskumar mentioned this pull request Sep 21, 2024
7 tasks
Copy link
Member

@frjo frjo left a comment

Choose a reason for hiding this comment

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

This needs a rebase. Would be good to get this in, it's the right approach I think.

@frjo frjo added Type: Feature This is something new (not an enhancement of an existing thing). Type: Minor Minor change, used in release drafter labels Jan 21, 2025
@theskumar
Copy link
Member Author

I think this needs to done fresh now, given the number of merge conflicts and new code that might have got added after.

@frjo frjo self-requested a review February 5, 2025 07:21
@theskumar theskumar marked this pull request as draft February 5, 2025 13:39
@theskumar theskumar force-pushed the submission-id-as-project-id branch 7 times, most recently from f3853bc to 8e14b0c Compare February 13, 2025 16:39
<p class="invoice-block__title">{% trans "Some details" %}</p>
<p class="invoice-block__meta">XXXXXX</p>
</li>
</ul>
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed this unused template.

{% trans "Are you sure you want to accept and approve the contract for commencing the project?" %}
{% trans "This cannot be undone." %}
</p>
{% url 'apply:projects:contract_approve' pk=object.submission.id as url %}
Copy link
Member Author

Choose a reason for hiding this comment

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

Minor copy and style update.

{% trans "Are you sure you want to submit contracting documents?" %}
{% trans "Make sure you have uploaded correct contract and all required contracting documents." %}
</p>
{% url 'apply:projects:contract_doc_upload' pk=object.submission.id as url %}
Copy link
Member Author

Choose a reason for hiding this comment

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

Minor style update of the modal text.

if user.is_superuser:
return True, "Superuser can view all documents"
if user == project.user:
return True, "Vendor can view all documents"
Copy link
Member Author

Choose a reason for hiding this comment

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

superuser and the applicant itself were not able to view the contracting documents uploaded. I've updated the code to fix it, please advise. @frjo @sandeepsajan0 @wes-otf

Copy link
Member Author

Choose a reason for hiding this comment

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

To reproduce, after uploading the additional documents in the contract, login as the applicant and try clicking on the "view" button.
Screenshot 2025-02-14 at 8  46 13@2x

Copy link
Member

@frjo frjo Feb 14, 2025

Choose a reason for hiding this comment

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

@theskumar This is a bug so your fix is correct. Applicant are the ones uploading these documents so they should be able to view them.

My guess this bug was introduced when we added the feature to optionally limit what internal groups can access these documents (staff, finance etc.).

Copy link
Member Author

Choose a reason for hiding this comment

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

ok

@theskumar theskumar marked this pull request as ready for review February 14, 2025 11:16
@theskumar
Copy link
Member Author

This became a bit bigger PR than I expected. Almost all of the public facing urls should be in the form of /apply/projects/<submission-id>/... now.

There are minor style update to models and a bug fix for contracting document visibility.

@frjo @wes-otf best way to test this out is to browse through the project section and try out all the flow.

frjo pushed a commit that referenced this pull request Feb 20, 2025
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.
@theskumar theskumar force-pushed the submission-id-as-project-id branch from f9e93d7 to 38781e7 Compare February 20, 2025 08:15
@frjo
Copy link
Member

frjo commented Feb 20, 2025

The edit invoice url still uses the project id.

Some test seems to need to be updated as well.

@theskumar
Copy link
Member Author

No worries. On it.

@theskumar theskumar force-pushed the submission-id-as-project-id branch 2 times, most recently from 42038c6 to ed7fa4b Compare March 1, 2025 15:49
@frjo
Copy link
Member

frjo commented Mar 9, 2025

@theskumar This needs a new rebase after your other PR that I have merged in.

After the rebase I plan to merge these three PR:s as well. Have tested them on test and they seem to work without issues.

@theskumar theskumar force-pushed the submission-id-as-project-id branch from ed7fa4b to 8ccbe0a Compare March 10, 2025 06:32
…roject (#4413)

Part 1 of:
- #4355

Depends on:
- #3944

Update the detail pages for submissions, communications, and projects to
use the same status header when an application has transitioned to a
project.

The header behavior mimics the status bar display for concept and
proposal applications, showing the proposal stages as the application
moves from the concept phase to the proposal phase.

It reuses the existing status bar generation template tag. DRY!

Also, enable the communication form if the submission is archived but
the project is active.

## Test Steps
- Verify that a user can leave a comment in archived submission mode if
it has an associated project, by navigating to the URL
/submissions/[id]/#communications.
- If an application has an associated project, ensure the project's
status header is displayed.
@theskumar
Copy link
Member Author

Ready for final review and merge.

I've merged in project header pr #4413 .

@frjo frjo removed Status: Needs testing Tickets that need testing/qa Status: Needs dev testing 🧑‍💻 Tasks that should be tested by the dev team labels Mar 11, 2025
@frjo frjo merged commit ae5dbf8 into main Mar 11, 2025
7 checks passed
frjo pushed a commit that referenced this pull request Mar 11, 2025
Depends on: 
- #3944
- #4413

What's changed: 
- project and submission to use the same communication page
- update urls to point to the new page
- update urls in email. 
- Cleanup the old comment view page code
- new permission check added "view_comments" -- anyone with either
access to project or submission will have access to it's comments page.

Other updates:
- Global activity feed has updated text for the comments
- New function based view is use to show and add comment
- Remove the use of tab.js on the submisison/project detail page, all
the tabs are server-rendered full-page now.

Fixes #4355
@theskumar theskumar deleted the submission-id-as-project-id branch March 11, 2025 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature This is something new (not an enhancement of an existing thing). Type: Minor Minor change, used in release drafter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants