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.

Sign up for GitHub

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

Jump to bottom

SALTO-6517: fix static file path collision #6518

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

SALTO-6517: fix static file path collision #6518

wants to merge 1 commit into from
+39 −12

Conversation

IdoZakk
Copy link
Contributor

@IdoZakk IdoZakk commented Sep 2, 2024

Fixed the static file path collision in Jira

  • It makes more sense that all adapters will do it similarly, so I added a function to adapter-utils and moved the okta solution to use it.
  • The solution is based on the solution agreed upon in Okta following a conversation with @netama
  • A change was done in jira to use the elemID name instead of the name in the value of the instance. It seems safer, as in some cases in Jira to API allows for two elements with the same name, and we can have different elemIDs for them (for instance if one of them was changed after starting to use Salto)

Added noise reduction

Release Notes:
Jira Adapter:

  • Fixed a potential bug with static file path for icons

User Notifications:
Jira Adapter:

  • The static file path of icons will change to avoid collisions

@coveralls
Copy link

Coverage Status

coverage: 93.953% (+0.01%) from 93.94%
when pulling 18c7b68 on IdoZakk:6517StaticFileCollision
into d0a1a10 on salto-io:main.

idoamit8
idoamit8 approved these changes Sep 3, 2024
View reviewed changes
Copy link
Contributor

@idoamit8 idoamit8 left a comment

Choose a reason for hiding this comment

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

Nice work! Thanks for consolidating the code 💯

@Nurdok Nurdok mentioned this pull request Sep 15, 2024
Merged
shoshana-houri
shoshana-houri reviewed Sep 29, 2024
View reviewed changes
packages/adapter-utils/src/nacl_case_utils.ts
Comment on lines +28 to +30
// Converts a nacl case to a file system safe name. Use it (instead of pathNaclCase) if the file name must be unique
export const fileNameNaclCase = (name: string): string => name.replace('@', '.')

Copy link
Contributor

Choose a reason for hiding this comment

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

Added it in this PR #6625
As suggested by @shir-reifenberg

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shoshana-houri shoshana-houri shoshana-houri left review comments

@idoamit8 idoamit8 idoamit8 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@IdoZakk @coveralls @idoamit8 @shoshana-houri