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

webuipoc: UI implementation to reactWeb #5460

Merged
merged 6 commits into from
Jul 18, 2024
Merged

Conversation

Moeez905
Copy link
Contributor

@Moeez905 Moeez905 commented May 17, 2024

Eslint configured in our Zap React App.

Issue Reference:
#8287.

@thc202 thc202 changed the title Eslint configured webuipoc: add eslint to reactWebUI May 17, 2024
@thc202 thc202 changed the title webuipoc: add eslint to reactWebUI [WIP] webuipoc: add eslint to reactWebUI May 17, 2024
@Moeez905 Moeez905 changed the title [WIP] webuipoc: add eslint to reactWebUI [WIP] webuipoc: add eslint and UI implementation to reactWeb May 22, 2024
@thc202
Copy link
Member

thc202 commented May 23, 2024

This needs to integrate the latest changes.

@Moeez905
Copy link
Contributor Author

Moeez905 commented Jun 3, 2024

Ready for review: did code cleanup, conflicts resolve and rebase.

@Moeez905
Copy link
Contributor Author

Moeez905 commented Jun 7, 2024

Ready for review: did code cleanup, conflicts resolve and rebase.

Conflict resolved - Ready for review

@Moeez905 Moeez905 changed the title [WIP] webuipoc: add eslint and UI implementation to reactWeb webuipoc: add eslint and UI implementation to reactWeb Jun 7, 2024
@Moeez905 Moeez905 changed the title webuipoc: add eslint and UI implementation to reactWeb webuipoc: UI implementation to reactWeb Jun 14, 2024
@thc202 thc202 mentioned this pull request Jun 14, 2024
@Moeez905
Copy link
Contributor Author

This PR is ready to review

@kingthorin
Copy link
Member

It has conflicts. Though that doesn't prevent review, but it'll need to be addressed before merge.

@Moeez905
Copy link
Contributor Author

It has conflicts. Though that doesn't prevent review, but it'll need to be addressed before merge.

Resolved them just now

@thc202
Copy link
Member

thc202 commented Jun 24, 2024

The build is failing.

@njmulsqb
Copy link
Contributor

The build is failing.

I am unable to understand the reason it's failing. npm i is working fine locally. Do you see any specific issue?

@kingthorin
Copy link
Member

Click checks, pick one of the failed jobs, look at the log.

@njmulsqb
Copy link
Contributor

Its stating that npm ci run failed, but why is it even executing at first place? It's not defined in package.json file.

@kingthorin
Copy link
Member

Probably a bad merge commit. You'd be better off rebasing and addressing conflicts.

@thc202
Copy link
Member

thc202 commented Jun 27, 2024

The ci is to do a clean install: https://docs.npmjs.com/cli/v10/commands/npm-ci worth running that locally to make sure all is correct.

@njmulsqb
Copy link
Contributor

njmulsqb commented Jul 4, 2024

Re-doing npm i solved the build failing problem.

@psiinon
Copy link
Member

psiinon commented Jul 8, 2024

I get the same error when trying to build this as per the CI build: 'npm ci' can only install packages when your package.json and package-lock.json or npm-shrinkwrap.json are in sync. Please update your lock file with npm install before continuing.
We cant review this PR unless it builds ☹️

@psiinon
Copy link
Member

psiinon commented Jul 9, 2024

Still failing to build :/

@Moeez905
Copy link
Contributor Author

Moeez905 commented Jul 9, 2024

Still failing to build :/

Fixed.

@kingthorin
Copy link
Member

Seems to have a ton of unrelated commits again.

@Moeez905
Copy link
Contributor Author

Moeez905 commented Jul 9, 2024

Seems to have a ton of unrelated commits again.

I'm struggling with rebase, can you tell me exact way or cmds to get this done

@kingthorin
Copy link
Member

kingthorin commented Jul 9, 2024

To provide commands to fix it I'll have to clone it later and try.

As for rebasing, the normal process is:

git fetch upstream
git rebase upstream/main
git push --force

@Moeez905
Copy link
Contributor Author

Moeez905 commented Jul 9, 2024

image
It will take forever to resolve conflicts and rebase; what's the best practice here, should I rebase after every few commits?

@psiinon
Copy link
Member

psiinon commented Jul 9, 2024

If I know no one else has changed the code I'm working on then I just do a mixed reset, not sure if thats good practice or not..

@kingthorin
Copy link
Member

I can't seem to figure out what you've done 😞

@Moeez905
Copy link
Contributor Author

Moeez905 commented Jul 9, 2024

There are so many conflicts :( I am trying to resolve in rebase for past 30 mins or so, any other solution I can try to fix this?

@thc202
Copy link
Member

thc202 commented Jul 9, 2024

Don't worry about the commits.

addOns/commonlib/CHANGELOG.md Outdated Show resolved Hide resolved
addOns/grpc/CHANGELOG.md Outdated Show resolved Hide resolved
addOns/openapi/CHANGELOG.md Outdated Show resolved Hide resolved
addOns/wappalyzer/CHANGELOG.md Outdated Show resolved Hide resolved
@psiinon
Copy link
Member

psiinon commented Jul 11, 2024

The build is failing due to changes in other add-ons ☹️

@Moeez905
Copy link
Contributor Author

The build is failing due to changes in other add-ons ☹️

any idea how to get this done , I've tried npm i and npm ci

@kingthorin
Copy link
Member

Nice work. Glad you got this passing again!

addOns/webuipoc/src/main/pocs/reactWebUI/eslint.config.mjs Outdated Show resolved Hide resolved
addOns/webuipoc/webuipoc.gradle.kts Outdated Show resolved Hide resolved
addOns/webuipoc/webuipoc.gradle.kts Outdated Show resolved Hide resolved
Moeez905 and others added 3 commits July 15, 2024 03:04
Signed-off-by: Moeez Ahmed <118293110+Moeez905@users.noreply.github.com>
Signed-off-by: Moeez Ahmed <118293110+Moeez905@users.noreply.github.com>
Signed-off-by: Moeez Ahmed <x311099@gmail.com>
@thc202
Copy link
Member

thc202 commented Jul 16, 2024

IMO the tree should look like more the Sites tree (or the example web UI, i.e. show path segments rather than the full URI). I guess it can be improved later.

Signed-off-by: Moeez Ahmed <x311099@gmail.com>
Signed-off-by: Moeez Ahmed <x311099@gmail.com>
@thc202 thc202 enabled auto-merge (squash) July 16, 2024 17:07
@thc202
Copy link
Member

thc202 commented Jul 16, 2024

Thank you!

@thc202 thc202 merged commit d25a15f into zaproxy:main Jul 18, 2024
10 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jul 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants