Internal: AI - Added session and generate ids [ED-12962]#24477
Merged
matipojo merged 22 commits intoelementor:mainfrom Dec 7, 2023
Merged
Internal: AI - Added session and generate ids [ED-12962]#24477matipojo merged 22 commits intoelementor:mainfrom
matipojo merged 22 commits intoelementor:mainfrom
Conversation
…lementor into ED-12962-new-add-relevant-ids
matipojo
commented
Dec 4, 2023
matipojo
commented
Dec 4, 2023
StyleShit
suggested changes
Dec 5, 2023
tests/jest/unit/modules/ai/assets/js/editor/layout-app-wrapper.test.js
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
file name kebab-case? (also elementorCommon.js)
| import { createTheme } from '@elementor/ui'; | ||
| import { StyledEngineProvider } from '@elementor/ui/styles'; | ||
| import { ThemeProvider } from '@mui/material/styles'; | ||
| import {} from '@mui/material'; |
Contributor
There was a problem hiding this comment.
Suggested change
| import {} from '@mui/material'; |
| import PropTypes from 'prop-types'; | ||
|
|
||
| // https://stackoverflow.com/a/77460953 - Thanks. | ||
| export default class TestProvider extends React.Component { |
Contributor
There was a problem hiding this comment.
Suggested change
| export default class TestProvider extends React.Component { | |
| export default class TestThemeProvider extends React.Component { |
or maybe
Suggested change
| export default class TestProvider extends React.Component { | |
| export default class TransitionlessThemeProvider extends React.Component { |
| contrastText: '#242105', | ||
| }, | ||
| }, | ||
| // Disable Transitions when running tests |
Contributor
There was a problem hiding this comment.
i would move this comment to the top, since it's related to this whole component
Comment on lines
+76
to
+78
| generateId: generateIdRef.current, | ||
| batchId, | ||
| requestId: `request-${ getUniqueId() }`, |
Contributor
There was a problem hiding this comment.
why those IDs are being handled here and not in the backend?
If I understand correctly, seems like the only client-dependant ID is the session, am I right?
Member
Author
There was a problem hiding this comment.
The backend is stateless, so it's blind to the batch to a regenerate.
…lementor into ED-12962-new-add-relevant-ids
matipojo
commented
Dec 6, 2023
modules/ai/assets/js/editor/pages/form-layout/hooks/use-screenshots.js
Outdated
Show resolved
Hide resolved
StyleShit
suggested changes
Dec 6, 2023
| assertUniqueIds( defaultExpectedUniqueIds ); | ||
| } ); | ||
|
|
||
| it( 'Should keep the same sessionId and generateID on regenerate, but discard other ids', async () => { |
Contributor
There was a problem hiding this comment.
shouldn't also have now "keep editorSessionId" or something?
Comment on lines
+111
to
+115
| await clickEditPromptButton(); | ||
|
|
||
| // Act - Should keep only the sessionId, on a new prompt. | ||
| await addPromptAndGenerate( 'test2' ); | ||
|
|
StyleShit
approved these changes
Dec 7, 2023
matipojo
added a commit
to matipojo/elementor
that referenced
this pull request
Dec 7, 2023
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.