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

Changed Source-parameter in editFormUrl #530

Merged
merged 4 commits into from
Oct 5, 2021
Merged

Conversation

siifux
Copy link
Contributor

@siifux siifux commented Sep 29, 2021

Description

I have changed the Url that is set as a &Source parameter to not include the query param, so that you won't be redirected back to the spesific status report you came from. I have read the examples written in the docs for Location, and I think this solution will grab the Url without the query/hash.

@siifux siifux self-assigned this Sep 29, 2021
@siifux siifux linked an issue Sep 29, 2021 that may be closed by this pull request
Copy link
Member

@tarjeieo tarjeieo left a comment

Choose a reason for hiding this comment

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

Strålende

@siifux siifux marked this pull request as ready for review October 5, 2021 08:11
@siifux siifux requested a review from olemp as a code owner October 5, 2021 08:11
Copy link
Collaborator

@olemp olemp left a comment

Choose a reason for hiding this comment

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

Looks good @siifux 🔪

@olemp olemp changed the title Changed Url-component in editFormUrl Changed Source-parameter in editFormUrl Oct 5, 2021
@olemp olemp merged commit 653449f into dev Oct 5, 2021
@olemp olemp deleted the feature/479-statusReport-focus branch October 5, 2021 11:56
@pzljanb pzljanb mentioned this pull request Oct 11, 2021
26 tasks
@pzljanb
Copy link
Contributor

pzljanb commented Oct 12, 2021

@siifux This is not working. It continues to catch to report you are "coming from" before you created a new report.
Can be tested here: CI Clean install environment
Report "New" Url

Please add a "How to test" section fr the next test :-)

@siifux
Copy link
Contributor Author

siifux commented Oct 12, 2021

@pzljanb I see.. The editForm url still contains the &Source param set to the selected report you came from.. @olemp I still haven't gotten to test this case easily while coding since the change is in the @shared project, so should find a way to do that?

@tarjeieo
Copy link
Member

Most likely the shared-package from npm is used, which is not current. Let's put effort into publishing the shared-package before investing any more time into this.

@olemp
Copy link
Collaborator

olemp commented Oct 12, 2021

Most likely the shared-package from npm is used, which is not current. Let's put effort into publishing the shared-package before investing any more time into this.

Working on creating a puzzlepart-user on npm that has access to the packages.

@olemp
Copy link
Collaborator

olemp commented Oct 12, 2021

Most likely the shared-package from npm is used, which is not current. Let's put effort into publishing the shared-package before investing any more time into this.

Working on creating a puzzlepart-user on npm that has access to the packages.

Sent an email to npm support. I had puzzlepart registered as an org on my user. It's now deleted but it seems to take some time before the scope becomes available.

For now (as a temp fix) I can give the other people on the PP team access with their npm users.

Just give me your usernames on npmjs.com and I'll fix it @siifux @tarjeieo @Remi749 @Rundez @TinHoangVu

@siifux
Copy link
Contributor Author

siifux commented Oct 15, 2021

Tested this, and works as expected with the npm packages updated 👏

@pzljanb
Copy link
Contributor

pzljanb commented Oct 15, 2021

Tested this, and works as expected with the npm packages updated 👏

Working well in both upgraded and fresh install

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.

Change focus to newly created status report
4 participants