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

Fixed issue #17862: Invalid url in admin (usage of HTTP_HOST) #2282

Merged

Conversation

gabrieljenik
Copy link
Collaborator

Compiling of JS is pending

@Shnoulle
Copy link
Collaborator

Shnoulle commented Mar 3, 2022

Strangely : «Add question» is OK but not «add group»

?

@gabrieljenik
Copy link
Collaborator Author

Strangely : «Add question» is OK but not «add group»

?

Sorry, don't understand.
What do you mean?

@Shnoulle
Copy link
Collaborator

Shnoulle commented Mar 7, 2022

report : Go to manage admin survey :

  1. Can create survey, can edit global settings a lot os OK
  2. Add button question in structure goes to limesurvey.intra
  3. Global survey settings tab goes all to limesurvey.intra

In structure : Add question is broken , not add group (i made an error in my report)

BUT : i can not reproduce today … on my local nginx …

Copy link
Contributor

@c-schmitz c-schmitz left a comment

Choose a reason for hiding this comment

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

Code is okay, but CI is not green

@gabrieljenik
Copy link
Collaborator Author

Compiling of JS is pending. I wasn't able to do it. Got errors all time. Asked @olleharstedt for help :)

@olleharstedt
Copy link
Collaborator

So it's finally happened. We need to apply a bug fix in adminsidepanel and the build system is FUBAR...

@olleharstedt
Copy link
Collaborator

I think Jessica tried to update to Vue 3, but you probably have to downgrade it to Vue 2 again to easily apply a new fix.

@gabrieljenik
Copy link
Collaborator Author

I think Jessica tried to update to Vue 3, but you probably have to downgrade it to Vue 2 again to easily apply a new fix.

I am sorry, we were not able to make it work yet.
Someone want to take it over?

@olleharstedt
Copy link
Collaborator

Jessica is back next week, maybe we'll assign it to her then. Or it will have to be a team effort.

Also, death to dependabot.

@olleharstedt
Copy link
Collaborator

I moved the task to our internal Zoho project, we'll try to solve it.

@naiteon
Copy link
Contributor

naiteon commented Mar 15, 2022

@gabrieljenik side menu has been fixed. If you find any problems reach out to me please.

@gabrieljenik
Copy link
Collaborator Author

@naiteon Thanks! What was it?
What are the next steps then? Should we merge master in here and recompile?

@olleharstedt
Copy link
Collaborator

Sorry, still broken. False alarm. :(

@gabrieljenik
Copy link
Collaborator Author

@naiteon Thanks! What was it?
What are the next steps then? Should we merge master in here and recompile?

@naiteon
Copy link
Contributor

naiteon commented Mar 18, 2022

@gabrieljenik Hey Gabriel! I think you should stash your changes, rebase your branch against master, then apply the stashed changes and proceed with the PR.

@naiteon
Copy link
Contributor

naiteon commented Mar 18, 2022

@gabrieljenik As for compiling the adminsidepanel, there is no need for recompiling it because the distribution files are included in the repo.

@olleharstedt
Copy link
Collaborator

@gabrieljenik As for compiling the adminsidepanel, there is no need for recompiling it because the distribution files are included in the repo.

This fix changes the adminsidepanel src files, that's why they need to be recompiled.

@gabrieljenik
Copy link
Collaborator Author

Ready for re review!

@Shnoulle Shnoulle added the Code review done Version checked for code issue without testing label Apr 25, 2022
@gabrieljenik
Copy link
Collaborator Author

I think this can be merged. Seems tested and also reviewed? Please someone elese confirm it. Thanks

@Shnoulle
Copy link
Collaborator

I think this can be merged. Seems tested and also reviewed? Please someone elese confirm it. Thanks

I don't have the exact process for merging ;) .
Maybe the reviewer can merge ?

@olleharstedt olleharstedt merged commit 9281e80 into LimeSurvey:master Apr 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code review done Version checked for code issue without testing Needs code review
Projects
None yet
7 participants