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

[E-Documents Core] - Manual multiple files import #8

Open
wants to merge 23 commits into
base: main
Choose a base branch
from

Conversation

petemchlk
Copy link
Collaborator

@petemchlk petemchlk commented Jan 22, 2025

Manual multiple files import

This pull request contains changes that make possible to upload multiple files at once if user choose to create E-Documents using manual Import Files action on E-Documents page.

  • New fileuploadaction "Import Files" added to E-Documents page (6122)
  • Action "New From File" hidden as its functionality is covered by new action
  • Changes in "E-Doc. Import" codeunit to handle import using FileUpload list

MS PR: microsoft#27975

EDocument: Record "E-Document";
EDocGetBasicInfo: Codeunit "E-Doc. Get Basic Info";
begin
// Commit before getting basic info with error handling (if Codeunit.Run then )
Copy link
Owner

Choose a reason for hiding this comment

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

No no, just one space symbol need to be removed before the last bracket :D

tinestaric
tinestaric previously approved these changes Jan 24, 2025
Copy link
Collaborator

@tinestaric tinestaric 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! Good work! :D

EDocGetBasicInfo: Codeunit "E-Doc. Get Basic Info";
begin
// Commit before getting basic info with error handling (if Codeunit.Run then)
Commit();
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know this is from another PR, but this commit in here makes me a bit nervous. Because seeing a procedure like HasDuplicates doesn't make you think that the transaction will be commited in here...

You don't have to change anything, because I know you have other fires to fight, and I know MS is quite liberal with their commits as well, but just something to think about...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually tried to avoid this commit, but happened to me while testing manually to get this error. Will keep this in my mind so if there will be some time left will try to handle this :)

tinestaric
tinestaric previously approved these changes Jan 24, 2025
@nerkel91
Copy link
Collaborator

Implementation

With this update users can generate multiple incoming E-Documents when manually uploading multiple attachments from vendor documents.
Users can now manually upload multiple attachments from the E-Documents (ID 6122) page using the new Import Files action. The system will generate an individual E-Document for each uploaded file.
If E-Document already exists with the same:

  • Incoming E-Document No

  • Bill-to/Pay-to (Vendor No)

  • Document Date

The system will generate a warning message to prevent duplication.

CurrPage.Update();
if EDocImport.ChooseEDocumentService(EDocumentService) then begin
EDocImport.UploadDocument(Rec, EDocumentService);
CurrPage.Update();
Copy link
Owner

Choose a reason for hiding this comment

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

Add explicit RunTrigger. Very recommend using false if that's enough for the solution

PurchaseHeader.DeleteAll(false);
DocumentAttachment.DeleteAll(false);

EDocument.DeleteAll();
Copy link
Owner

Choose a reason for hiding this comment

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

EDocument.DeleteAll(true); or EDocument.DeleteAll(false);

GMatuleviciute
GMatuleviciute previously approved these changes Feb 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants